Community
Participate
Working Groups
+++ This bug was initially created as a clone of Bug #384906 +++ SWT Tree also has header, so same set of APIs as added for bug 384906 also needs to be added in Tree, for consistency and completeness sake.
The code provided for Bug 384906 works also for Tree headers.
(In reply to Conrad Groth from comment #1) > The code provided for Bug 384906 works also for Tree headers. Can you create a Gerrit for Windows for it? I can afterwards add the GTK implementation. We should try to finish this before M6 as this is our last milestone in which we can do API changes without PMC approval.
I added a Gerrit patch just with the API.
(In reply to Conrad Groth from comment #3) > I added a Gerrit patch just with the API. We need the changes in ControlExample's TreeTab as well to verify/test the proposed patch.
(In reply to Niraj Modi from comment #4) > (In reply to Conrad Groth from comment #3) > > I added a Gerrit patch just with the API. > > We need the changes in ControlExample's TreeTab as well to verify/test the > proposed patch. I added only the API for all platforms, because API freeze for Oxygen is soon. There is no implementation for any platform. I will add that, after the patch for the Table was merged, because it's basically the same code.
New Gerrit change created: https://git.eclipse.org/r/91822
(In reply to Conrad Groth from comment #5) > (In reply to Niraj Modi from comment #4) > > (In reply to Conrad Groth from comment #3) > > > I added a Gerrit patch just with the API. > > > > We need the changes in ControlExample's TreeTab as well to verify/test the > > proposed patch. > > I added only the API for all platforms, because API freeze for Oxygen is > soon. There is no implementation for any platform. I will add that, after > the patch for the Table was merged, because it's basically the same code. Ok, but we cannot add new APIs to SWT without implementation, at-least implementation on one platform is must to make it to M6.
(In reply to Niraj Modi from comment #7) > (In reply to Conrad Groth from comment #5) > > (In reply to Niraj Modi from comment #4) > > > (In reply to Conrad Groth from comment #3) > > > > I added a Gerrit patch just with the API. > > > > > > We need the changes in ControlExample's TreeTab as well to verify/test the > > > proposed patch. > > > > I added only the API for all platforms, because API freeze for Oxygen is > > soon. There is no implementation for any platform. I will add that, after > > the patch for the Table was merged, because it's basically the same code. > Ok, but we cannot add new APIs to SWT without implementation, at-least > implementation on one platform is must to make it to M6. Moving out of M6. We still have a chance to get the Tree header APIs into M7 but only after verification followed by PMC approval.
Tested both the patches combined, noticed few problems: Win7: - foreground/background coloring works fine on Tree header only when Tree header is visible - Try setting the Tree header's background/foreground first and then turn the Tree header visible & Multiple columns check-boxes on. No coloring applies to Tree header. Ubuntu: - am getting compilation error highlighted in the patch. ControlExample: - I see icons before header background and header foreground option missing. [Become visible only when colors are set] JavaDoc: New API JavaDocs needs to be addressed for all platform according to the feedback given on bug 513405 JUnits: Also we will need to have JUnits against the new APIs, suggest we add them as separate gerrit patch or it can also be part of ControlExampe patch.
(In reply to Niraj Modi from comment #9) > Tested both the patches combined, noticed few problems: > Win7: > - foreground/background coloring works fine on Tree header only when Tree > header is visible > - Try setting the Tree header's background/foreground first and then turn > the Tree header visible & Multiple columns check-boxes on. No coloring > applies to Tree header. This is the method org.eclipse.swt.examples.controlexample.TreeTab.setExampleWidgetState(): It's missing 'setHeaderBackground (); setHeaderForeground ();' directly after 'setCellFont ();' > > Ubuntu: > - am getting compilation error highlighted in the patch. Fixed that. > > ControlExample: > - I see icons before header background and header foreground option missing. > [Become visible only when colors are set] Same cause like above in the TreeTab class. > > JavaDoc: > New API JavaDocs needs to be addressed for all platform according to the > feedback given on bug 513405 Fixed that. > > JUnits: > Also we will need to have JUnits against the new APIs, suggest we add them > as separate gerrit patch or it can also be part of ControlExampe patch. I will make a separate patch.
(In reply to Conrad Groth from comment #10) > (In reply to Niraj Modi from comment #9) > > Tested both the patches combined, noticed few problems: > > Win7: > > - foreground/background coloring works fine on Tree header only when Tree > > header is visible > > - Try setting the Tree header's background/foreground first and then turn > > the Tree header visible & Multiple columns check-boxes on. No coloring > > applies to Tree header. > This is the method > org.eclipse.swt.examples.controlexample.TreeTab.setExampleWidgetState(): > It's missing 'setHeaderBackground (); setHeaderForeground ();' directly > after 'setCellFont ();' > > > > > Ubuntu: > > - am getting compilation error highlighted in the patch. > Fixed that. > > > > > ControlExample: > > - I see icons before header background and header foreground option missing. > > [Become visible only when colors are set] > Same cause like above in the TreeTab class. > > > > > JavaDoc: > > New API JavaDocs needs to be addressed for all platform according to the > > feedback given on bug 513405 > Fixed that. > > > > > JUnits: > > Also we will need to have JUnits against the new APIs, suggest we add them > > as separate gerrit patch or it can also be part of ControlExampe patch. > I will make a separate patch. I confirm above issues are resolved. Tested the patch on Win7, it looks good. Few improvement in JavaDocs needs to be addressed see gerrit: https://git.eclipse.org/r/#/c/91422 Hi Lars, I don't see the expected behavior on GTK, it seems only stubs are present. We need to implement the Tree header on GTK as well on same lines as you did in bug 509648. Once we have all the patches in place, then only we can seek PMC approval. Thanks!
(In reply to Niraj Modi from comment #11) > (In reply to Conrad Groth from comment #10) > I confirm above issues are resolved. Tested the patch on Win7, it looks good. > Few improvement in JavaDocs needs to be addressed see gerrit: > https://git.eclipse.org/r/#/c/91422 > > Hi Lars, > I don't see the expected behavior on GTK, it seems only stubs are present. > We need to implement the Tree header on GTK as well on same lines as you did > in bug 509648. > > Once we have all the patches in place, then only we can seek PMC approval. > Thanks! Hi everyone: I implemented the GTK functionality, as was implemented in bug 509648. I tested it out, and seems to work without issue. There is one important note: I noticed that in ControlExample, for Table the "multiple columns" box is checked by default. For Tree it isn't, meaning that if you try to set the header background/foreground without checking it first, you will get a NPE since no columns exist. This would affect Table too, so I added a null check in both Tree and Table to prevent this case.
Niraj, could you please review this with priority? We in Platform UI need to provide the CSS styling implementation based on the SWT change and the css for the dark theme. We really would like to have this in for M7 so that we can deliver (finally) a (relative) decent dark theme.
To avoid delays in this I will ask already for PMC approval for this change. Of course, this change needs still the final code review by the SWT team but as it is almost the same as the already integrated change for table headers, I expect no surprises in this code review. And I would like to avoid missing M7 due to missing PMC approval.
(In reply to Lars Vogel from comment #14) > To avoid delays in this I will ask already for PMC approval for this change. > > Of course, this change needs still the final code review by the SWT team but > as it is almost the same as the already integrated change for table headers, > I expect no surprises in this code review. And I would like to avoid missing > M7 due to missing PMC approval. NOTE: PMC approval depends on whether this change is accepted by SWT.
+1 from me. Verified/Tested latest patch both the patches combined on Win7 and a quick round of verification on Ubuntu16.04 as well.
(In reply to Niraj Modi from comment #16) > +1 from me. > Verified/Tested latest patch both the patches combined on Win7 and a quick > round of verification on Ubuntu16.04 as well. Are you going to push it now or you want me to do it?
(In reply to Alexander Kurtakov from comment #17) > (In reply to Niraj Modi from comment #16) > > +1 from me. > > Verified/Tested latest patch both the patches combined on Win7 and a quick > > round of verification on Ubuntu16.04 as well. > > Are you going to push it now or you want me to do it? Both the patches are now in master. JUnits/Examples patch was manually merged to master via below git commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=0311cc0637c61d3d24e0d166c4c80fa4679b51cf
Marking as fixed, Till(Brychcy) can you open a bug for Cocoa
Marking as fixed, Bug 514424 handles Cocoa.
