This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
| Summary: | The editor toolbar should prefer the TopComponent lookup as the context for context-aware actions | ||
|---|---|---|---|
| Product: | editor | Reporter: | Andrei Badea <abadea> |
| Component: | -- Other -- | Assignee: | Andrei Badea <abadea> |
| Status: | RESOLVED FIXED | ||
| Severity: | blocker | CC: | apireviews |
| Priority: | P2 | Keywords: | API_REVIEW_FAST |
| Version: | 5.x | ||
| Hardware: | All | ||
| OS: | All | ||
| Issue Type: | DEFECT | Exception Reporter: | |
| Attachments: |
Proposed fix
Proposed fix incl. tests Action context is composed of the node and Lookup.Provider lookups |
||
|
Description
Andrei Badea
2006-03-28 16:35:00 UTC
Created attachment 29452 [details]
Proposed fix
As Mila's suggestion I would like to ask for a review of this change. I am attaching the implementation and some tests proving the implementation is correct. Created attachment 29486 [details]
Proposed fix incl. tests
Am I correct that this would in fact permit the toolbar to be attached to an editor window that is not a TopComponent at all, provided that some container implements Lookup.Provider? If so, +1. Though it looks like that may already be the case...? does it work in multiviews as well? jglick: I think this was already possible (although I haven't tried). But I think the fact that you don't have the lookup of the enclosing Lookup.Provider available somehow limits this use case. mkleint: no, it doesn't work in multiviews :-( The first Lookup.Provider is a CloneableEditor having only a o.o.w.DelegateActionMap in its lookup, but no nodes. Thank you for the catch! It seems the action context lookup should be composed from the first Lookup.Provider's lookup and the lookup of the node delegate corresponding to the current document. The implementation has to ensure that the resulting lookup doesn't contain the node delegate twice -- see the new diff. Created attachment 29495 [details]
Action context is composed of the node and Lookup.Provider lookups
The while-loop can be simplified using Collection.contains(). Lookups.exclude might be helpful, I don't know. Re. loop: sure. It's true what they say, too much copy-pasting is bad... I will fix it before committing. Re. Lookups.exclude: I have considered it shortly, but I prefer the current approach unless there is some problem with it. It seems cleaner to me that I don't exclude anything from the lookups, I either return one of them or an union of both. If there are no more comments, I would like to integrate by the end of the week. Thank you for the review. Integrated into the trunk: Checking in src/org/netbeans/modules/editor/NbEditorToolBar.java; /cvs/editor/src/org/netbeans/modules/editor/NbEditorToolBar.java,v <-- NbEditorToolBar.java new revision: 1.25; previous revision: 1.24 done RCS file: /cvs/editor/test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java,v done Checking in test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java; /cvs/editor/test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java,v <-- NbEditorToolBarTest.java initial revision: 1.1 done Integrated into the release55 branch: Checking in nbproject/project.properties; /cvs/editor/nbproject/project.properties,v <-- project.properties new revision: 1.17.6.1.2.1; previous revision: 1.17.6.1 done Checking in src/org/netbeans/modules/editor/NbEditorToolBar.java; /cvs/editor/src/org/netbeans/modules/editor/NbEditorToolBar.java,v <-- NbEditorToolBar.java new revision: 1.18.2.1.2.1; previous revision: 1.18.2.1 done Checking in test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java; /cvs/editor/test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java,v <-- NbEditorToolBarTest.java new revision: 1.1.2.1; previous revision: 1.1 done |