Community
Participate
Working Groups
We can style CTabFolders but not CTabItems. Note that CTabItem is a widget but not a control so this might make supporting a bit trickier. See the CSS support for Gallery v.s. GalleryItem for an example of how we handled this. PROPERTIES - showClose - font - image PSEUDO SELECTORS - :selected (see bug #263186)
Added bug #263186 as being blocked by this bug (once we add the CTabItem support we can them move :selected out of CTabFolder and into here).
Kevin, won't 'image' be an issue (for some parts) since the image changes from part to part? Also, do we think that it's necessary to be able to have two different items in the same stack have different close button settings? Could we make this a CTabFolder attribute (as it currently is)?
(In reply to comment #2) > Kevin, won't 'image' be an issue (for some parts) since the image changes from > part to part? First off note that if the CSS doesn't specify an image then there's no conflict with programatically setting one. So adding this support won't hurt our dynamic usage. From an SWT/CSS point of view it may make sense to have it settable from CSS (e.g. a simple RCP app where all the tabs have the same image). But a more sophisticated and true CSS approach would be that the modelled UI annotates the CTabItems with a CSS class based on information about say the view type in the model, and there is a CSS expression which says which tab image to use. For example, CTabItem.markersView { image: ... } CTabItem.outlineView { image: ... } I'm not suggesting that though as it then ignores the images supplied in the 3.x extension points but it's worth thinking about in a 'pure' e4 world. It'd be pretty cool though :) > Also, do we think that it's necessary to be able to have two different items in > the same stack have different close button settings? Could we make this a > CTabFolder attribute (as it currently is)? Right, I was just going down the list of setters and admit I forgot about CTabFolder.setUnselectedCloseVisible() which we support today in CSS, and the use of SWT.CLOSE as a create arg for CTabFolder. So, we probably don't need showClose for any practical usage, and besides it doesn't do anything anyway if you use SWT.CLOSE on create of the CTabFolder.
(In reply to comment #3) > (In reply to comment #2) ... > > Also, do we think that it's necessary to be able to have two different items in > > the same stack have different close button settings? Could we make this a > > CTabFolder attribute (as it currently is)? > > Right, I was just going down the list of setters and admit I forgot about > CTabFolder.setUnselectedCloseVisible() which we support today in CSS, and the > use of SWT.CLOSE as a create arg for CTabFolder. > > So, we probably don't need showClose for any practical usage, and besides it > doesn't do anything anyway if you use SWT.CLOSE on create of the CTabFolder. OTOH, the following does look like the "right" way to express the views/editors close box policy: CTabFolder.editors CTabItem { showClose: true; } CTabFolder.views CTabItem { showClose: false; } CTabFolder.views CTabItem:selected { showClose: true; } We should code up a simple example to at least see if it works (ie. don't use SWT.CLOSE or setUnselectedCloseVisible()).
One issue with the CTabItem showClose approach is that it may result in a tab item appearing, /then/ getting styled with the close box either being there by default and then disappearing, or the opposite. Another example of why we need SWT support so styling can be applied just prior to widget appearance.
(In reply to comment #5) > One issue with the CTabItem showClose approach is that it may result in a tab > item appearing, /then/ getting styled with the close box either being there by > default and then disappearing, or the opposite. Another example of why we need > SWT support so styling can be applied just prior to widget appearance. In some preliminary stand alone tests this seems to work ok so we should proceed with the CSS implementation.
Created attachment 133030 [details] CTabItem added as a selector This patch adds CTabItem as a selector, and changes the pseudo selected so that it is no longer CTabFolder: selected, but CTabItem:selected. The stylable properties for CTabItem are: - font-size - font-family - font-style - color - background-color - background-image - show-close Appropriate Junit tests are also in the patch
This patch needs a lot more work: 1. It's easy to get various class cast exceptions using the CSSSWTEditor. 2. CSSPropertyFontSWTHandler.applyCSSProperty() doesn't have cases for all the property types (e.g. font-variant), which can result in a null key, which will blow up at folder.setData(key, listener). 3. CSSPropertyFontSWTHandler for the case of CTabItem does not benefit from all the code in it's super class which handles all the font properties. Instead, it attempts to duplicate this code in applyTabItemFont(), so I'm not sure we're handling all the cases. 4. I am suspicious of the fact there's no corresponding retrieveTabItemFont() method.
Removing milestone since nobody working on this atm
Created attachment 139714 [details] CTabItem fonts patch v1 Instead of tackling all the things that attachment 133030 [details] tries to solve, I've only attacked the font problem for now (in addition to ignoring the "selected" selector business). The patch is my take of it after considering the comments from comment 8. One thing I noticed about attachment 133030 [details] is that its SWTElement implementation returns the child CTabItems in the item(int) call. From what I gather, this effectively means that any child controls of the CTabFolder (the content being displayed under a tab) will _not_ get styled during the recursion process. I've replaced it with a more cumbersome implementation that includes both child controls and child items although I think it might be a little (too) fragile. I've considered this child control styling business in the attached tests. (In reply to comment #8) > 1. It's easy to get various class cast exceptions using the CSSSWTEditor. What exactly is the CSSSWTEditor? I don't seem to have this class in my workspace.
Thanks for the patch Remi! (In reply to comment #10) > Instead of tackling all the things that attachment 133030 [details] tries to solve, I've > only attacked the font problem for now (in addition to ignoring the "selected" > selector business). The patch is my take of it after considering the comments > from comment 8. OK I'm fine with staging it up (with the eventual goal of writing CTabItem:selected). > One thing I noticed about attachment 133030 [details] is that its SWTElement > implementation returns the child CTabItems in the item(int) call. From what I > gather, this effectively means that any child controls of the CTabFolder (the > content being displayed under a tab) will _not_ get styled during the recursion > process. Good catch yes. > What exactly is the CSSSWTEditor? I don't seem to have this class in my > workspace. It's the example org.eclipse.e4.ui.examples.css. The patch seems good and it's not generating the errors the previous patch did. Tests run, works in the editor too. This is kind of fun. In the editor example I hacked to mark ".busy" and ".modified" on the two CTabItems respectively. So we can now write: CTabItem.busy { font: italic; } CTabItem.modified { font: bold; } Cool! Discussion: I have (minor) mixedfeelings about the CSSSWTFontHelper methods. We've changed the arg type from Control to Widget, so that we'll have a common supertype for Control and CTabItem, but not all Widget's have fonts. The code works fine with the right helping of instanceof's but it seems a bit misleading. I guess it's ok (in theory we could associate a font with other non-controls via lookup table or something). Aside: the next property of CTabItem I'd like to see is show-close in combination with selected:, so we can write: CTabItem { show-close: false; } CTabItem:selected { show-close: true; } CTabFolder.editorStack CTabItem { show-close: true; } which we could make immediate use of. Presently that code is buried in the renderer.
Committed patch v1 plus mod of example to include .busy .modified. We'll need to update the examples since CTabFolder { font: ... } no longer does anything. Leaving bug open with next to do :selected show-close image
(In reply to comment #11) > > What exactly is the CSSSWTEditor? I don't seem to have this class in my > > workspace. > > It's the example org.eclipse.e4.ui.examples.css. I will check it out from CVS later, thanks. > Discussion: I have (minor) mixedfeelings about the CSSSWTFontHelper methods. > We've changed the arg type from Control to Widget, so that we'll have a common > supertype for Control and CTabItem, but not all Widget's have fonts. The code > works fine with the right helping of instanceof's but it seems a bit > misleading. I guess it's ok (in theory we could associate a font with other > non-controls via lookup table or something). Originally I had the instanceof checks in CSSPropertyFontSWTHandler but then I realized I had to apply the same for the reverse process (retrieval) so I pushed it to CSSSWTFontHelper. Unfortunately, Items don't have Fonts or Colors and it's only in some of its concrete subclasses (like CTabItem, TableItem, TreeItem, etc.) I wonder if it'd make sense to spin a new subclass of AbstractCSSPropertyFontHandler that addresses subclasses of Item? I guess then we'd have to introduce a lot of similar handlers for other attributes. > Aside: the next property of CTabItem I'd like to see is show-close in > combination with selected: I have an idea about how to implement this involving listeners but to not have "hard coded values" (which causes problems like bug 279349). Could we attach selection listeners to the tab folder and then when an item is selected we query the engine for values? If we get proper values, we style the tab item. If null is returned (implying that the last-parsed style sheet did not define CTabItem as a selector), we revert the values to SWT defaults. Does that make sense? // not real code of course handleEvent(Event e) { Style s = engine.getStyle(e.widget); if (s == null) { revert(e.widget); } else { s.style(e.widget); } } Of course, we will use a dispose listener to remove this listener when the widget is disposed. Do you think this will work, Kevin? > CTabFolder.editorStack CTabItem { > show-close: true; > } I don't think the current renderers distinguish MParts between editors and views, do we? I'm not up to speed on the consensus but I do believe there's been a lot of discussions about this on other bugs and probably offline as well? (In reply to comment #12) > Committed patch v1 plus mod of example to include .busy .modified. Okelydokely.
(In reply to comment #13) > Originally I had the instanceof checks in CSSPropertyFontSWTHandler but then I > realized I had to apply the same for the reverse process (retrieval) so I > pushed it to CSSSWTFontHelper. Unfortunately, Items don't have Fonts or Colors > and it's only in some of its concrete subclasses (like CTabItem, TableItem, > TreeItem, etc.) I wonder if it'd make sense to spin a new subclass of > AbstractCSSPropertyFontHandler that addresses subclasses of Item? I guess then > we'd have to introduce a lot of similar handlers for other attributes. Yeah I recognized all that, hence I shrugged and went "ok" :) (In reply to comment #12) > We'll need to update the examples since > CTabFolder { > font: ... > } > > no longer does anything. I think I misspoke. It appears to still work from the tests, contacts and photo examples. But for reasons I'm not sure of, this *doesn't* seem to work in the SWT Editor example: CTabFolder { font-size: 14; } (no styling change). Needs more investigation. Will also reply to rest of comment #13 later.
(In reply to comment #14) > (In reply to comment #12) > > We'll need to update the examples since > > CTabFolder { > > font: ... > > } > > > > no longer does anything. > > I think I misspoke. It appears to still work from the tests, contacts and photo > examples. In thinking about this more, CTabFolder should continue to work as it did. CTabFolder supports setFont(), so it's quite legal to continue to use a font: property in CTabFolder, as well as now in CTabItem. The former sets the font on all the items. > But for reasons I'm not sure of, this *doesn't* seem to work in the SWT Editor > example: > > CTabFolder { > font-size: 14; > } > > (no styling change). This continues to perplex me.
If a stylesheet simply defines... CTabFolder:selected { show-close: true } ...then when the selection switches, do we automatically switch all other tabs to setShowClose(false)? Or should we store the last selected tab and its state and then do a revert? However, would this be a little unpredictable if the application has code sprinkled around that explicitly called setShowClose(true)? Is this something that developers should worry about or should they use their own CTabItem CSS classes/IDs and let the designer design the style sheet. I suppose this is in line with our master plan to take over the world...I mean, uh...allow Eclipse to penetrate into other domains and markets and let people write applications more easily, yes...that... I'm currently hacking up an implementation of comment 13 and this seems to be one of the flaws with that approach. (In reply to comment #15) > > But for reasons I'm not sure of, this *doesn't* seem to work in the SWT Editor > > example: > > > > CTabFolder { > > font-size: 14; > > } > > > > (no styling change). > > This continues to perplex me. If it'll make you feel any better I can reproduce this on my computer. :o
Based on my print statements, the Font from getFont() is indeed changed. However, the items themselves do not seem to have changed in size.
Examples problem solved! The bug is in AbstractCSSEditor's applyStyle(Object) method. It seems that invoking getCSSEngine() is no good. Instantiating a new CSSSWTEngineImpl every go will fix the CTabFolder font size problem. I guess this is an alternate incarnation of bug 278479.
(In reply to comment #16) > If a stylesheet simply defines... > > CTabFolder:selected { show-close: true } As a minor note, it would be CTabItem { show-close: true } and meanwhile CTabFolder { unselected-close-visible: true } (blech!) but your question is still valid. > ...then when the selection switches, do we automatically switch all other tabs > to setShowClose(false)? Or should we store the last selected tab and its state > and then do a revert? However, would this be a little unpredictable if the > application has code sprinkled around that explicitly called > setShowClose(true)? Is this something that developers should worry about or > should they use their own CTabItem CSS classes/IDs and let the designer design > the style sheet. This is definitely a general issue. It's quite easy for the application and css to "fight". Ideally, you'd not try to style the same attributes from both. The CSS support tries to solve this by providing the ability to capture the widget state prior to applying the style sheet. See CSSEngine.applyStyles(Object, boolean, boolean), where the 2nd boolean is computeDefaultStyle. This doesn't work that great though, since all it can do is capture the widget state prior to applying the style sheet and believe *that's* the default state. But app code which changes the widget state after styling doesn't get capture in this. There is discussion in removing this, making 'default state' == 'widget constructed state'. See bug #278412 for discussion, which I see you are CC'd on. > > This continues to perplex me. > > If it'll make you feel any better I can reproduce this on my computer. :o I'm going to look at this today.
Created attachment 139930 [details] CTabItem "selected" selector / show-close patch v01 This is a first cut attempt at the implementation outlined in comment 13. Kevin, I pulled the CSSPropertyShowCloseSWTHandler class as well as the testShowClose() test in CTabFolderTest. Is this what you had in mind or should I have left those in? This patch does _not_ alter the rendering or compatibility code. To test the stylesheet outlined in comment 11, please use the main method below. The left side is a view stack and the right side is an editor stack. Display display = new Display(); final CSSEngine engine = new CSSSWTEngineImpl(display); try { engine.parseStyleSheet(new StringReader( "CTabItem { show-close: false; }" + "CTabItem:selected { show-close: true; }" + "CTabFolder.editors CTabItem { show-close: true; }")); } catch (IOException e) { display.dispose(); e.printStackTrace(); return; } final Shell shell = new Shell(display); shell.setLayout(new FillLayout()); final CTabFolder viewsFolder = new CTabFolder(shell, SWT.NONE); for (int i = 0; i < 5; i++) { CTabItem item = new CTabItem(viewsFolder, SWT.NONE); item.setText("View " + Integer.toString(i)); } final CTabFolder editorsFolder = new CTabFolder(shell, SWT.NONE); SWTElement.setCSSClass(editorsFolder, "editors"); for (int i = 0; i < 5; i++) { CTabItem item = new CTabItem(editorsFolder, SWT.NONE); item.setText("Editor " + Integer.toString(i)); } engine.applyStyles(shell, true); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose();
(In reply to comment #20) > Created an attachment (id=139930) [details] > CTabItem "selected" selector / show-close patch v01 I forgot to note that fonts are _not_ supported with the selector. We set the fonts in CSSPropertyFontSWTHandler's onAllCSSPropertiesApplyed(Object, CSSSEngine) method which has no pseudo parameters. I don't think CSS2FontProperties stores any pseudo information either, do they? Without the pseudo information, I'm not sure if the same engine retrieval tactic can be employed here. :|
Created attachment 139932 [details] CTabItem "selected" selector / show-close patch v02 This patch enables font support with the "selected" selector although I don't know if this is the right way to go about it. One big problem we have is that CTabFolder's setSelection(...) APIs doesn't fire any SWT.Selection events...which kind of makes this patch useless. I am unable to find other ways to capture setSelection(...) calls at the moment.
(In reply to comment #11) > CTabItem { > show-close: false; > } > CTabItem:selected { > show-close: true; > } > CTabFolder.editorStack CTabItem { > show-close: true; > } We'll need to change the renderer or the stylesheet definition needs to change (to use IDs?) because StackModelFactory's createWidget(MPart<?>) method explicitly alternates the CTabFolders' class names to "active" and "inactive".
Created attachment 140002 [details] CTabItem "selected" selector / show-close patch v03 This v03 patch uses Kevin B's suggested SWT.Paint listener + selection check method. (In reply to comment #22) > One big problem we have is that CTabFolder's setSelection(...) APIs doesn't > fire any SWT.Selection events...which kind of makes this patch useless. I am > unable to find other ways to capture setSelection(...) calls at the moment. <rcjsuen> It sounds like a bug...but at the same time, I think if it was it would've been reported already and I couldn't find anything in bugzilla. I take it it's a "by design" thing. <krbarnes> it's by design. No setters in swt will trigger events. In theory, if you are calling the setter, you know that the change is happening and you can react on you own (ie without waiting for an event) <rcjsuen> and like button's setSelection method doesn't fire events either...so <rcjsuen> well, this looks like a toughie then <krbarnes> rcjsuen: there have been requests to change this, but we can't change the behavior after so many years. <rcjsuen> yeah, figures <krbarnes> rcjsuen: there was a request for a second set of listeners that were triggered always. I can't find it in bugzilla at the moment so maybe it was a wontfix. <rcjsuen> krbarnes: That sounds like a pretty big change <krbarnes> rcjsuen: can't you just setFont everytime you setSelection? <rcjsuen> krbarnes: the developer can ask the engine to restyle everytime they setSelection and/or use notifyListeners I suppose, yes <krbarnes> you could use some other event (Paint?) and track selection, if (selection != getSelectionFromTabFolder()) update() <rcjsuen> krbarnes: Paint might work. I'm concerned about the excessive calls but I suppose if we make sure to only update on change could be okay. <rcjsuen> I'll change my listener to SWT.Paint and check. <krbarnes> rcjsuen: For now that's the best suggestion I have. Is this going to be an issue for all widgets, or specific to CTabFolder? Maybe we can build support into SWT? <rcjsuen> krbarnes: We haven't gotten that far yet. I guess it'll be an issue for TabFolder...? ;p <rcjsuen> I guess Tree and Table might suffer the same problems. <rcjsuen> Hm I must've added my paint listener incorrectly. It never gets called during my unit test ;) <krbarnes> styling native widgets is going to be harder... on mac especially. <krbarnes> guess I'll leave it you to file an enhancement request when you know more about requirements. <rcjsuen> krbarnes: thanks for the help <rcjsuen> krbarnes: Great, SWT.Paint with selection test seems to be fine. It was working visually but not in my unit tests, it seems the paint events weren't processed but adding while (folder.getDisplay().readAndDispatch()); after setSelection(...) got them passing. Thanks! :) <krbarnes> rcjsuen: glad I could help
Font styles seems to be getting "merged" at the moment, I believe this is because they don't support pseudo. The test below will fail on the second line. CTabFolder folder = createTestTabFolder("CTabItem { font-style: italic }\n" + "CTabItem:selected { font-weight: bold }"); for (int i = 0; i < folder.getItemCount(); i++) { CTabItem item = folder.getItem(i); FontData fd = item.getFont().getFontData()[0]; if (item == folder.getSelection()) { assertEquals(SWT.BOLD, fd.getStyle()); } else { assertEquals(SWT.ITALIC, fd.getStyle()); } } The returned value is actually 3, which implies a value of (SWT.BOLD | SWT.ITALIC) instead of simply SWT.ITALIC.
(In reply to comment #25) Actually, due to inheritance, the selected font should both be bolded and italicized and then the regular font should just be italicized. There are still some problems caused by classes, like trying to use "CTabFolder.editors", since the font querying doesn't have this information.
(In reply to comment #25) > Font styles seems to be getting "merged" at the moment, I believe this is > because they don't support pseudo. I'm not sure the right solution pattern, but perhaps should check in applyCSSProperty() and if pseudo doesn't match state then don't call super, don't return true. Otherwise patch looks ok although clearly the listener feels pretty hacky. Not sure what else to suggest other than new CTabFolder which I want to write anyway :)
(In reply to comment #26) > (In reply to comment #25) > Actually, due to inheritance, the selected font should both be bolded and > italicized and then the regular font should just be italicized. Yes that's true, weight and style combine. How about: CTabItem { font-weight: normal; font-style: italic; } CTabItem:selected { font-weight: bold; }
Created attachment 140038 [details] CTabItem "selected" selector / show-close patch v04 Okay, the merged styles problem has been solved. It seems the CSS2FontProperties were getting "ignored" because we were setting (merged) Fonts in the handler's onAllCSSPropertiesApplyed(Object, CSSEngine) method. Since fonts are now completely managed by the listener (or at least they should be), I've turned it to a no-op if the widget is a CTabItem. CTabItem { font-weight: normal; font-style: italic } CTabItem:selected { font-weight: bold } ...is covered by testSelectedFontMerged() and... CTabItem { font-style: italic } "CTabItem:selected { font-weight: bold } ...is covered by testSelectedFontMerged2(). Everything except testClassFontsThisWillFail() _should_ pass. The failure is caused by the classes business noted in comment 25. So close, yet so far... :o
Created attachment 140044 [details] CTabItem "selected" selector / show-close patch v05 (In reply to comment #27) > I'm not sure the right solution pattern, but perhaps should check in > applyCSSProperty() > > and if pseudo doesn't match state then don't call super, don't return true. I don't completely understand this, could you rephrase that? (In reply to comment #29) > Created an attachment (id=140038) [details] > CTabItem "selected" selector / show-close patch v04 > Everything except testClassFontsThisWillFail() _should_ pass. The failure is > caused by the classes business noted in comment 25. Okay, v04's test was bad (failing at the wrong place). I've corrected it to fail at the right place. The problem seems to be because of merged fonts (and possibly in addition to the use of a parent element though testShowCloseViewStack() passes).
(In reply to comment #30) > Created an attachment (id=140044) [details] > CTabItem "selected" selector / show-close patch v05 > > (In reply to comment #27) > > I'm not sure the right solution pattern, but perhaps should check in > > applyCSSProperty() > > > > and if pseudo doesn't match state then don't call super, don't return true. > > I don't completely understand this, could you rephrase that? I don't have the code in front of me (its late) but I thought it worked like: Each time a specific property is set (font-size, weight, ...) a call is made to applyCSSProperty(..., String propName, String pseudo, ...) which calls super which dispatches to specific methods for each propName. These accumulate font changes in the CSS2FontProperties. Finally, onAllCSSPropertiesApplyed acts like a post processing step, taking the accumulated font information and actually applying it as one font change to the widget. Thus I thought the problem might be that we need to check on each property setting whether or not we should allow it because the widget is in the state matching the pseudo. By the time we get to onAllCSSPropertiesApplyed we've computed the new font info as an aggregate of all the individual property settings. Maybe they can be a mixed set of pseudo states for a given call to onAllCSSPropertiesApplyed. In any case it seems wasteful to compute all the font info if the widget isn't in the right state to begin with. The state check should be in the apply, not in the onAll, that's the thought.
Created attachment 140089 [details] CTabItem "selected" selector / show-close patch v06 The tests have been corrected (I keep forgetting about CSS inheritance) and I've reorganized the listener code a little to reduce code duplication. Since the tests are dependent on paint events, they are a little fragile and can fail if the stars do not align (window management, Alt+Tabbing around, etc). Kevin, if this one is fine then I'll push it to HEAD and close this. I propose _new_ and individual bugs be opened for every other property that the CTabItem element should expose as I think this bug has been goin' on long enough. ;)
(In reply to comment #32) > Created an attachment (id=140089) [details] > CTabItem "selected" selector / show-close patch v06 > > The tests have been corrected (I keep forgetting about CSS inheritance) and > I've reorganized the listener code a little to reduce code duplication. Since > the tests are dependent on paint events, they are a little fragile and can fail > if the stars do not align (window management, Alt+Tabbing around, etc). > > Kevin, if this one is fine then I'll push it to HEAD and close this. I propose > _new_ and individual bugs be opened for every other property that the CTabItem > element should expose as I think this bug has been goin' on long enough. ;) Had a brief review, looks ok. Once we get proper CTabItem support we'll likely remove :selected from CTabFolder, and the properties "unselected-close-visible", "unselected-image-visible" since the proper way to do them would be via CTabItem:selected.
(In reply to comment #33) > (In reply to comment #32) > > Kevin, if this one is fine then I'll push it to HEAD and close this. > Had a brief review, looks ok. (go ahead and release, Remi)
FYI, just added bug #282575 - [CSS] CTabItem should support color, background-color
(In reply to comment #34) > (go ahead and release, Remi) Dunno if McQ or Paul told you but I'm out of office / in Hong Kong / no Eclipse access until Friday evening (which is when M5 is due I believe?). Could you release this instead because I presume we don't want to push this out during RC1?
Hi Remy, I will be happy to release it for you. Kevin
Released patch 2009-06-25
