Bugzilla – Full Text Bug Listing
| Summary: | [CSS] Provide the CSS support for defining the new font and color definitions | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Daniel Rolka <daniel.rolka> |
| Component: | UI | Assignee: | Daniel Rolka <daniel.rolka> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, gheorghe, john.arthorne, pwebster |
| Version: | 4.4 | ||
| Target Milestone: | 4.4 M5 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=419016 | ||
| Whiteboard: | |||
| Attachments: | |||
|
Description
Daniel Rolka
Created attachment 237500 [details]
Style sheet using during tests
Created attachment 237501 [details]
Definitions added via CSS and assigned to default category
Created attachment 237502 [details]
Definitions added via CSS and added to the specified category
(In reply to Daniel Rolka from comment #0) > It is the extension for the Bug 419016 and the requirement: > > "If we added extra properties, like eclipse-font-name and > eclipse-font-category-id we could even allow defining new font definitions > in the CSS" > > Currently we process the CSS in one direction - having the widget we find > the proper style for it using the CSSEngine. It will require some > modification in the engine to add support for the opposite direction - > reviewing all defined CSS styles we find proper widget. > > Currently the font and color definitions defined with the > 'org.eclipse.ui.themes' extension point use label and description. The > question is how to support it with the CSS style sheet. What about the > globalization of strings? I've pushed the patch proposal for the functionality above to Gerrit: https://git.eclipse.org/r/#/c/18449/ To define the new font and color definitions the user has to add special element to the stylesheet called 'ThemesExtension' and put there ids of adding definitions like in the attachment 'Style sheet using during tests' It allows to skip the current implementation of the CSS engine and does not break the performance. Other attachments present the behavior of patch. When category is not specified in the stylesheet then the definition is adding to the default one. The same story is with labels and descriptions - when it is skipped then the defaults are used. The current patch does not support the globalisation of strings. I believe it will be fixed using different bug thanks, Daniel This looks like a good starting usecase. I reviewed https://git.eclipse.org/r/#/c/18449/3 and the applied the following: I used the Orion CSS pref editor to add this to the default style sheet: ThemesExtension { font-definition: "#org-eclipse-ui-workbench-FONT_DEF_2"; color-definition: "#org-eclipse-ui-workbench-COLOR_DEF_1"; } FontDefinition#org-eclipse-ui-workbench-FONT_DEF_2{ font-family: 'Arial'; font-size: 15; font-style: normal; category: "#org-eclipse-egit-ui-GitTheme"; label: "New font 2 added by CSS"; description: "The font 2 has been added by CSS"; } ColorDefinition#org-eclipse-ui-workbench-COLOR_DEF_1{ color: red; category: "#org-eclipse-egit-ui-GitTheme"; label: "New color 1 added by CSS"; description: "The color 1 has been added by CSS"; } Then I hit apply and OK. When I re-opened the Appearance pref page it looks like the new CSS is still there. But if I expand the Git category on the color and font page I can't see the new color or font like you see in comment #3. PW Created attachment 238351 [details] After running Eclipse with CSS from #5 (In reply to Paul Webster from comment #5) > When I re-opened the Appearance pref page it looks like the new CSS is still > there. But if I expand the Git category on the color and font page I can't > see the new color or font like you see in comment #3. > > PW I'm not able to recreate the issue you have described. It works fine for me (see attachment). Maybe there is some difference in the eGit's category id on you site. Please try without category id in the CSS. Then the new definitions will be assigned to the default category (Other added by CSS). Basically I apply the changes in CSS manually and next start/restart the Eclipse. So if you try to apply changes in fly then it won't work. If it is your case I think we can add such appendix to the functionality, however if you agree I would like to push these changes without it and add the 'apply changes in the runtime' extension using separate bug (the current patch contains several changes and it has started to be quite hard to keep up-to-date with the master) Btw. I've updated the patch in Gerrit with the proper unit tests thanks, Daniel I think we can push this once the last few comments are answered and keep working on it in new gerrit patches. I've added a jface unit test in that depends on your changeset. https://git.eclipse.org/r/19923 Can we add more unit tests at that level? I expect that when we switch the CSS theme, we 1) pick up any font and color overrides from the JFace registries. Could we verify that? And 2) that we can see new font and colors defined through the CSS in the jface registries (and they go away switching back themes). See org.eclipse.ui.tests.themes.JFaceThemeTest.testColorOverride() (although actually it's a font override test, not a color override test) PW (In reply to Paul Webster from comment #7) > I think we can push this once the last few comments are answered and keep > working on it in new gerrit patches. > > I've added a jface unit test in that depends on your changeset. > https://git.eclipse.org/r/19923 > > Can we add more unit tests at that level? I expect that when we switch the > CSS theme, we 1) pick up any font and color overrides from the JFace > registries. Could we verify that? And 2) that we can see new font and > colors defined through the CSS in the jface registries (and they go away > switching back themes). See > org.eclipse.ui.tests.themes.JFaceThemeTest.testColorOverride() (although > actually it's a font override test, not a color override test) > > PW The current version of the patch doesn't support the switching of the CSS theme in the runtime (using the IThemeEngine.setTheme api). Let me modify the patch respectively to enable it Without this change we can not update the considered unit tests Daniel Gerrit review https://git.eclipse.org/r/#/c/18449/ has been eleased to master as: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a65bccad7395914a88571f7a889e844db51e969c Unless I overlooked it, you forgot to get approval for the new dependencies. Please provide the link to the CQs if I missed them or remove those dependencies until proper approval is in place. For details see https://wiki.eclipse.org/Development_Resources/IP/Test_and_Build_Dependencies Also, I'm pretty sure this is the reason that the UI tests no longer run (see bug 425980). BTW: Before starting to use Mockito you should probably wait until bug 403676 is resolved. I've opened https://dev.eclipse.org/ipzilla/show_bug.cgi?id=7866 ... the 3 libraries are covered under piggyback CQs PW Released as: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a65bccad7395914a88571f7a889e844db51e969c Daniel Verified in the build: eclipse-SDK-I20140120-2000 Daniel |