Community
Participate
Working Groups
Build Identifier: Latest release / Juno The Eclipse Tabbed Properties Framework does not support tab-decorations i.e. dynamic images on tabs or dynamically changing color of a tab label. Tab labels are always displayed in black-color. An image can be added to a tab using propertyTabs extension in plugin.xml; the image is displayed when the tab is in selected state otherwise it is not displayed. But the image is not dynamic enough because one cannot programatically control whether an image should be displayed or not. This fix can be used to improve usability of tabbed properties UI because tab-decorations can be used to convey, at a glance, information about current state of UI on a tab. The fix should make a provision for possibly setting different number of images for different tabs. Reproducible: Always
Created attachment 219083 [details] Proposed fix for this defect
Created attachment 219084 [details] Information about the proposed fix
Created attachment 219085 [details] Information about the proposed fix
Hi Amit, I'd like to have a quick look at your patch, but right now it includes a lot of extraneous changes. It looks like you did use git to generate your patch, http://git.eclipse.org/c/platform/eclipse.platform.ui.git which is good, but it includes too much stuff. Could you please re-generate your patch? PW
Created attachment 219163 [details] Proposed fix for this defect Thanks Paul for helping me with recreating the right patch. Please review the changes and let me know your comments.
Created attachment 219462 [details] Proposed fix for this defect Note: This patch should be applied on top of patch for another bug 386417 (A tab label in Tabbed Properties View is cut-off towards right-side).
Created attachment 219463 [details] Information about the proposed fix
Created attachment 219600 [details] Proposed fix for this defect Added one more protected method viz getCurrentSelection() to TabbedPropertySheetPage class.
Created attachment 219601 [details] Information about the proposed fix
(In reply to comment #8) > Created attachment 219600 [details] > Proposed fix for this defect The use of this interface needs to be included in the static and dynamic JUnit tests for the Tabbed Properties view. We'll need an updated patch for that. PW
Created attachment 220489 [details] Proposed fix for this defect Removed ISectionDescriptorProvider2 from the earlier patch because it is not necessary anymore. Rolled back the changes in TabbedPropertyRegistry because those changes are not needed any more after ISectionDescriptorProvider2 has been removed. Note: Apply this patch on top of the patch for another bug 386417 in TabbedPropertyList class (Title: A tab label in Tabbed Properties View is cut-off towards right-side).
Created attachment 220490 [details] Information about the proposed fix
Created attachment 220491 [details] Added / modified JUnit tests for bug fixes 386417 and 385795 I have included JUnit tests for bug fixes 386417 and 385795 in one single patch because I could not figure out how to separate those changes in my Git environment. Sorry for the inconvenience this may cause. These two bug-fixes are interdependent, so it seemed okay to combine JUnit tests for both. To try out these changes, apply the patches in following sequence: 1. Apply patch for another bug 386417 in TabbedPropertyList class (Title: A tab label in Tabbed Properties View is cut-off towards right-side) 2. Apply patch for proposed fix for this bug#385795 3. Apply patch for JUnit tests for bug fixes 386417 and 385795 Added a new helper method named getWidestLabelIndex() to TabbedPropertyList class. This method is used only in JUnit tests. Following changes test fix for bug 386417: 1. New GIF icons for error, information and warning 2. Changes in plugin.xml to display these icons on corresponding tabs 3. Three new tests in TabbedPropertySheetPageTest suite. One of these tests, test_widestLabelIndex3(), fails in the absence of fix for bug 386417. Other changes test fix for bug 385795.
Created attachment 220690 [details] Information about the proposed fix
Created attachment 220691 [details] Code changes that address the review comments. This patch should be applied on top of the three patches described in the previous comment. Apply the patches in following sequence: 1. Apply patch for another bug 386417 in TabbedPropertyList class (Title: A tab label in Tabbed Properties View is cut-off towards right-side) 2. Apply patch for proposed fix for this bug#385795 3. Apply patch for JUnit tests for bug fixes 386417 and 385795 4. Apply patch that addresses the review comments
I've pushed your combined changes up to a topic branch in our git repo, that's what we'll review against. Please give it a quick once over, I followed the steps in comment #15 and all elements appear to be there. It will be visible on git.eclipse.org and http://github.com/eclipse. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=869e212de39a901e2165957835e7c9bc478c4bdc PW
(In reply to comment #16) > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=869e212de39a901e2165957835e7c9bc478c4bdc Overall, it looks good. 1) @@ -606,7 +607,10 @@ public class TabbedPropertySheetPage } } - private void disposeTabs(Collection tabs) { + /** + * @since 3.6 + */ + protected void disposeTabs(Collection tabs) { when you make this protected, you should update the javadoc to describe any pre/post conditions. ex, the tabs this method disposes must not include the current tab (or it has to have been reset before this method is called). Also the disposed tabs have to somehow be removed from descriptorToTab (or if they're all disposed, it needs to be reset), like org.eclipse.ui.views.properties.tabbed.TabbedPropertySheetPage.updateTabs(ITabDescriptor[]) or org.eclipse.ui.views.properties.tabbed.TabbedPropertySheetPage.disposeContributor() 2) The tests are fine and check tab widths. But you need 2 or 3 tests that test adding the images and seeing that they're used correctly, showDynamicImage and hide, and setTextColor. Could you try and base your changes off of the commit above? Either a patch against that, or a commit from a fork at http://github.com/ PW
Created attachment 220847 [details] Code changes that address additional review comments in #17 I verified the contents on pwebster/bug385795 branch. This patch has been created on top of those changes, it addresses review comments #17. Do we need to add @since annotation for newly added/changed methods in TabbedPropertyList and TabbedPropertyList.ListElement classes? Changes in this patch: 1. Added null check in setTextColor() method of TabbedPropertyList.ListElement 2. Added JavaDoc to disposeTabs() method in TabbedPropertySheetPage class 3. Modified the test-class TabbedPropertySheetPageWithDecorations to invoke newly added methods in TabbedPropertyList.ListElement
Created attachment 220848 [details] Code changes that address additional review comments in #17 Added a couple more JavaDoc comments to TabbedPropertyList class.
Released as part of http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=3bef50ed015f031ff19d9fe68c054f050f12c1c6 PW
