Menu
▾
▴
Re: [Wonder-cvs] SF.net SVN: wonder:[11659] branches/Wonder_5_0_0_WebObjects_5_4_Branch/ Wonder/Frameworks/Core/ERDirectToWeb/Sources/er/directtoweb/components/ buttons/ERDControllerButton.java
|
From: Ramsey G. <ra...@xe...> - 2010-11-11 14:13:44
|
That's cool. Thanks Denis.
On Nov 11, 2010, at 8:54 AM, Denis Frolov wrote:
> I've just reverted the change. Sorry that it took 9 days instead of 3
> as promised.
>
> On Tue, Nov 2, 2010 at 11:26 AM, Denis Frolov <df...@de...>
> wrote:
>> On Tue, Nov 2, 2010 at 12:23 AM, Ramsey Gurley <ra...@xe...>
>> wrote:
>>>
>>> On Nov 1, 2010, at 4:40 PM, Denis Frolov wrote:
>>>
>>>> On Mon, Nov 1, 2010 at 6:41 PM, Ramsey Gurley
>>>> <ra...@xe...> wrote:
>>>>>
>>>>> On Nov 1, 2010, at 10:58 AM, Denis Frolov wrote:
>>>>>
>>>>>> Thanks for reviewing this, Ramsey. I was hoping Anjo would
>>>>>> comment on
>>>>>> this as well.
>>>>>>
>>>>>> I assumed it was a bug because the previous comment was "find
>>>>>> the next
>>>>>> non-null NextPageDelegate in the component tree, break if there
>>>>>> is a
>>>>>> D2WPage found beforehand". The "break" part seems to be saying
>>>>>> that we
>>>>>> don't want to use the controller of the root page for the
>>>>>> embedded
>>>>>> page.
>>>>>
>>>>> I think the comment there is a copy paste artifact from the
>>>>> ERDActionBar's
>>>>> method by the same name. That one grabs the topmost
>>>>> nextPageDelegate()
>>>>> that
>>>>> is a branch delegate. I think it does that because of the way
>>>>> D2WEmbeddedComponent's actionPageDelegate works.
>>>>> ERDControllerButton
>>>>> works
>>>>> by grabbing the first pageController moving up the parent chain,
>>>>> so you
>>>>> can
>>>>> override one delegate with another if you need.
>>>>
>>>> Honestly, I thought that ERDActionBar should also use the nearest
>>>> branch delegate even if it is null. Sorry, I lost you on the
>>>> "D2WEmbeddedComponent's actionPageDelegate" thing. What is the
>>>> connection here?
>>>
>>> Setting the nextPageDelegate via rules on an embedded page isn't
>>> possible if
>>> the action is bound because the component that does the embedding
>>> will
>>> automatically create at ActionPageDelegate to fire the bound
>>> action instead.
>>> Before discovering this, I had wondered why ActionBar always went
>>> to the
>>> top nextPageDelegate instead of using the nearest one. I am
>>> speculating that
>>> the actionPageDelegate behavior may have played a part in that
>>> design
>>> decision. It's pretty unlikely that you'll have a branch delegate
>>> as the
>>> NextPageDelegate in your nested pages. I could be totally wrong
>>> though
>>> (^_^) You'd have to ask Anjo why he did it that way.
>>>
>>>>
>>>>>> I saw the rule from BugTracker and assumed that it means "Use
>>>>>> only
>>>>>> "view" branch from the controller if we are on embedded page".
>>>>>> BugTracker's controllers are assigned on a per-entity basis, so
>>>>>> it
>>>>>> would be strange to use e.g. Bug's controller from root page for
>>>>>> embedded Requirement page.
>>>>>
>>>>> Looking at the rules, I think somewhere will be an ERDList of
>>>>> test items,
>>>>> and because of that rule, a view action will appear on the right
>>>>> controllerAction. I think with this change, there will be a null
>>>>> delegate
>>>>> and probably a null pointer exception if that action is
>>>>> selected. I'm
>>>>> just
>>>>> guessing at that by looking at the rules though.
>>>>
>>>> I've just checked BugTracker. There is an embedded list of test
>>>> items
>>>> on the home page and it works since there is a rule:
>>>>
>>>> 50 : entity.name = 'TestItem' => pageController =
>>>> er.bugtracker.delegates.TestItemDelegate
>>>> [er.directtoweb.ERDDelayedObjectCreationAssignment]
>>>>
>>>> Which makes perfect sense to me. It would be counter intuitive to
>>>> use
>>>> pageController of root page since it could be anything - e.g. a
>>>> pageController of another entity. But I guess this depends on your
>>>> usage pattern of pageControllers.
>>>>
>>>>>> I'll double check if anything is broken in BugTracker. Do you
>>>>>> actually
>>>>>> use the "one controller for all the pages" pattern in your
>>>>>> projects?
>>>>>> I'm all for reverting if this is indeed a feature...
>>>>>>
>>>>>> Denis
>>>>>
>>>>> I was working on something this weekend actually (^_^) I'm
>>>>> putting
>>>>> together
>>>>> a default branch delegate that I hope I can use on all my root
>>>>> pages. It
>>>>> has methods like _edit(...) _inspect(...) _removeRelated(...)
>>>>> _editRelated(...) _delete(...) and so on for all the common CRUD
>>>>> ops that
>>>>> take place on pages. Since the method names are underscored,
>>>>> they will
>>>>> have
>>>>> to be named explicitly in a branchChoices rule to show up in the
>>>>> interface
>>>>> and won't cascade down. Then for the branchChoices rule, I will
>>>>> create
>>>>> an
>>>>> assignment that will look at the state of the d2wContext to
>>>>> determine
>>>>> what
>>>>> default choices should be available and when.
>>>>>
>>>>> My goal is to remove all the control code (all the buttons) from
>>>>> my pages
>>>>> and encapsulate it in the default branch delegate. That way
>>>>> it's all in
>>>>> one
>>>>> place if I need to debug and I can switch out my buttons with
>>>>> one rule in
>>>>> the rule system when I need a custom override. At the same time,
>>>>> I hope
>>>>> to
>>>>> eliminate the need for a lot of keys like shouldShowCancelButton,
>>>>> shouldShowSubmitButton, shouldShowWhateverButton. Plus, if I
>>>>> need new
>>>>> buttons, I can simply subclass my default delegate, plug it in
>>>>> with a
>>>>> rule,
>>>>> and the default assignment should still work for sub-sub-pages...
>>>>> automatically ignoring any non-underscored method names (^_^)
>>>>
>>>> This should work with page-specific controllers (ones that we
>>>> have now
>>>> - after my commit) as well. You'll just need a rule like:
>>>>
>>>> *true* => pageController = DefaultBranchDelegate.
>>>
>>> Or just override the branchDelegate() method on my subclasses to
>>> use the
>>> original behavior (^_^) I don't actually use the
>>> ERDControllerButton
>>> anywhere, I just subclass it. Which brings up another possibility
>>> here.
>>> Instead of changing the way the existing component works, there
>>> could simply
>>> be a new component added to do this. Problem solved.
>>>
>>>> You will have multiple instances of delegate (one for each page),
>>>> but
>>>> is this a problem really? I doubt it would save you a lot of
>>>> memory.
>>>> As for the common place for vars - I've never used page
>>>> controller to
>>>> store vars, but you could store the vars in the top-most controller
>>>> anyway by accessing it via e.g. context.page or a cover method
>>>> somewhere.
>>>>
>>>> Again, the problem of the previous setup is that you can get very
>>>> unexpected delegates. Here is a problem that we had in our project:
>>>>
>>>> ListComments page is embedded to InspectPost page. InspectPost page
>>>> has PostController with Post-specific method, e.g. "publish"
>>>> which is
>>>> not applicable to Comment. ListComments has no controller because
>>>> it
>>>> doesn't need one on UI. This leads to a problem because
>>>> ListComments
>>>> gets PostController as a delegate and "publish" method as a
>>>> result of
>>>> that. Of course, I could override the PostController with a dummy
>>>> ListComments controller with no methods, but this seemed to be a
>>>> bit
>>>> wrong to me.
>>>
>>> Underscore method names can prevent that. If these really are
>>> entity
>>> specific methods, you could also override defaultBranchChoices()
>>> in your
>>> delegate like several of the bugtracker delegates do.
>>>
>>> protected NSArray defaultBranchChoices(D2WContext context) {
>>> NSArray result = super.defaultBranchChoices(context);
>>> if(!context.entity().name().equals("Post")) {
>>> result = NSArray.emptyArray();
>>> }
>>> return result;
>>> }
>>
>> I agree, that there are numerous ways to solve the problem. It's just
>> I think they are a bit dirty and break OO principles. E.g. super-
>> class
>> BranchDelegate should not know anything about subclasses and possible
>> entities.
>>
>>> Ok, now that I've heard your use case, consider this one. Suppose
>>> that you
>>> have an arrangement where comments _are_ posts via a self
>>> referential
>>> relationship and you _do_ want to cascade down. You could nest
>>> down with no
>>> end... Turtles all the way down (^_^) Should there be hundreds,
>>> possibly
>>> thousands of delegates on a single page?
>>
>> No, there will still be a single delegate instance for each d2w page
>> instance. The same way there is a single d2w-context on each page.
>>
>>>
>>>> Hopefully, Anjo will chime in;)
>>>
>>> I suspect if you want a reaction from Anjo, you'll need to commit
>>> this on
>>> the 5.3 branch... er, trunk (^_^)
>>
>> I doubt this would break anything in his projects seeing that
>> BugTracker uses delegate per entity pattern. We have numerous
>> projects
>> using pageControllers as well - and they are all fine.
>>
>> Anyway, if Anjo doesn't comment on this within 3 days, I'll roll-back
>> the commit and will move the logic to our custom subclass.
>>
>>>>
>>>> Dennis
>>>>
>>>>> Ramsey
>>>>>
>>>>>>
>>>>>> On Mon, Nov 1, 2010 at 5:06 PM, Ramsey Gurley
>>>>>> <ra...@xe...>
>>>>>> wrote:
>>>>>>>
>>>>>>> I was under the impression that this is a feature, not a bug.
>>>>>>> Having a
>>>>>>> single controller on a page has a number of benefits, from
>>>>>>> having a
>>>>>>> common place to stash private variables to simple reducing
>>>>>>> memory
>>>>>>> usage. Also, I haven't tried it, but it would appear this
>>>>>>> change will
>>>>>>> break bugtracker. Looking in the rules for bugtracker I see the
>>>>>>> pageController is only assigned by entity.name and
>>>>>>>
>>>>>>> 80: parentPageConfiguration != null => branchChoices =
>>>>>>> ({"branchButtonLabel" = "View"; "branchName" = "view"; })
>>>>>>> [Assignment]
>>>>>>>
>>>>>>> Ramsey
>>>>>>>
>>>>>>> On Nov 1, 2010, at 5:55 AM, df...@us... wrote:
>>>>>>>
>>>>>>>> Revision: 11659
>>>>>>>> http://wonder.svn.sourceforge.net/wonder/?rev=11659&view=rev
>>>>>>>> Author: dfrolov
>>>>>>>> Date: 2010-11-01 09:55:13 +0000 (Mon, 01 Nov 2010)
>>>>>>>>
>>>>>>>> Log Message:
>>>>>>>> -----------
>>>>>>>> Returning page controller from the closest page even if it is
>>>>>>>> null.
>>>>>>>> Looking for non-null controller can lead to unwanted usage of
>>>>>>>> delegate from the wrong entity in case of embedding.
>>>>>>>>
>>>>>>>> Modified Paths:
>>>>>>>> --------------
>>>>>>>> branches/Wonder_5_0_0_WebObjects_5_4_Branch/Wonder/Frameworks/
>>>>>>>> Core/ERDirectToWeb/Sources/er/directtoweb/components/buttons/
>>>>>>>> ERDControllerButton.java
>>>>>>>>
>>>>>>>> Modified: branches/Wonder_5_0_0_WebObjects_5_4_Branch/Wonder/
>>>>>>>> Frameworks/Core/ERDirectToWeb/Sources/er/directtoweb/
>>>>>>>> components/
>>>>>>>> buttons/ERDControllerButton.java
>>>>>>>> =
>>>>>>>> =
>>>>>>>> =
>>>>>>>> =
>>>>>>>> ===============================================================
>>>>>>>> --- branches/Wonder_5_0_0_WebObjects_5_4_Branch/Wonder/
>>>>>>>> Frameworks/
>>>>>>>> Core/ERDirectToWeb/Sources/er/directtoweb/components/buttons/
>>>>>>>> ERDControllerButton.java 2010-10-31 20:36:37 UTC (rev
>>>>>>>> 11658)
>>>>>>>> +++ branches/Wonder_5_0_0_WebObjects_5_4_Branch/Wonder/
>>>>>>>> Frameworks/
>>>>>>>> Core/ERDirectToWeb/Sources/er/directtoweb/components/buttons/
>>>>>>>> ERDControllerButton.java 2010-11-01 09:55:13 UTC (rev
>>>>>>>> 11659)
>>>>>>>> @@ -48,14 +48,15 @@
>>>>>>>> return css;
>>>>>>>> }
>>>>>>>>
>>>>>>>> - /** find the next non-null NextPageDelegate in the
>>>>>>>> component
>>>>>>>> tree, break if there is a D2WPage found beforehand */
>>>>>>>> + /** find the page controller of the closest D2WPage in the
>>>>>>>> component tree */
>>>>>>>> public ERDBranchDelegateInterface branchDelegate() {
>>>>>>>> if(branchDelegate == null) {
>>>>>>>> WOComponent current = parent();
>>>>>>>> - while(current != null && branchDelegate == null) {
>>>>>>>> + while(current != null) {
>>>>>>>> if(current instanceof ERD2WPage) {
>>>>>>>> ERD2WPage page = (ERD2WPage)current;
>>>>>>>> branchDelegate = page.pageController();
>>>>>>>> + return branchDelegate;
>>>>>>>> }
>>>>>>>> current = current.parent();
>>>>>>>> }
>>>>>>>>
>>>>>>>>
>>>>>>>> This was sent by the SourceForge.net collaborative development
>>>>>>>> platform, the world's largest Open Source development site.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ------------------------------------------------------------------------------
>>>>>>>> Nokia and AT&T present the 2010 Calling All Innovators-North
>>>>>>>> America
>>>>>>>> contest
>>>>>>>> Create new apps & games for the Nokia N8 for consumers in
>>>>>>>> U.S. and
>>>>>>>> Canada
>>>>>>>> $10 million total in prizes - $4M cash, 500 devices, nearly
>>>>>>>> $6M in
>>>>>>>> marketing
>>>>>>>> Develop with Nokia Qt SDK, Web Runtime, or Java and Publish
>>>>>>>> to Ovi
>>>>>>>> Store
>>>>>>>> http://p.sf.net/sfu/nokia-dev2dev
>>>>>>>> _______________________________________________
>>>>>>>> Wonder-cvs mailing list
>>>>>>>> Won...@li...
>>>>>>>> https://lists.sourceforge.net/lists/listinfo/wonder-cvs
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ------------------------------------------------------------------------------
>>>>>>> Nokia and AT&T present the 2010 Calling All Innovators-North
>>>>>>> America
>>>>>>> contest
>>>>>>> Create new apps & games for the Nokia N8 for consumers in
>>>>>>> U.S. and
>>>>>>> Canada
>>>>>>> $10 million total in prizes - $4M cash, 500 devices, nearly
>>>>>>> $6M in
>>>>>>> marketing
>>>>>>> Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to
>>>>>>> Ovi
>>>>>>> Store
>>>>>>> http://p.sf.net/sfu/nokia-dev2dev
>>>>>>> _______________________________________________
>>>>>>> Wonder-cvs mailing list
>>>>>>> Won...@li...
>>>>>>> https://lists.sourceforge.net/lists/listinfo/wonder-cvs
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
|