rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dan Gornstein <...@gornstein.com>
Subject Re: Angular Branch: Adding hasRegionWidgets method to PageResource
Date Fri, 13 Sep 2013 10:59:40 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message