rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Erin Noe-Payne <erin.noe.pa...@gmail.com>
Subject Re: Angular Branch: Adding hasRegionWidgets method to PageResource
Date Fri, 13 Sep 2013 16:16:24 GMT
Agreed with Dan.

On Fri, Sep 13, 2013 at 6:59 AM, Dan Gornstein <dan@gornstein.com> wrote:
> On Thu, Sep 12, 2013 at 7:53 PM, Rohit Kalkur <rohit.kalkur@gmail.com>wrote:
>
>> Hey everyone,
>>
>> I am writing a check for the ng-hide directive of the "Add widgets to this
>> page" message (which is displayed on any Page that does not have any
>> widgets on it).
>>
>> For now, I have added a method to the PageResource object called
>> hasRegionWidgets(), which basically determines whether any of the regions
>> for that page have region widgets.
>>
>> However, I was examining the tabs.html and thought eventually it might make
>> sense to refactor the CurrentPageCtrl into 2 separate controllers: one for
>> handling the current page tab functionality (edit page, move page, delete
>> page, etc...) and one for managing operations on the current page itself
>> (sending the current page info to the widget store).
>>
>
> +1 I think this makes sense. We don't want the controllers to get too big
> and handle too much so that they can be reused by people for specific
> purposes. These seem like two different parts of functionality and I
> believe make sense to be split up.
>
>>
>> If we go with that design, it might make sense to move the
>> 'hasRegionWidgets check' to the controller that would manage the current
>> page operations.
>>
>
> +1
>
>>
>> Anyhow, I just wanted to poll the dev list to see what everyone's thoughts
>> were. For now, I was thinking of just submitting the patch with the
>> hasRegionWidgets() method on the PageResource and then later on down the
>> road if we go with the 2 split controllers, we can just refactor
>> everything.
>>
>
> I think submitting the patch for now is fine as we are in active
> development and it is easy to change once some more people weigh in on the
> options or conform to lazy consensus.
>
>>
>> Let me know what you think.
>>
>> Thanks!
>> -Rohit K
>>

Mime
View raw message