Bugzilla – Full Text Bug Listing
| Summary: | [Table] Hover/selection foreground color overrides cell color | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Markus Krüger <webmaster> | ||||||
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> | ||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | m.kempe, neubauer, tbuschto | ||||||
| Version: | 1.4 | ||||||||
| Target Milestone: | 1.5 M7 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | 364969 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
Hi Markus, I've checked your example. The inheritance works fine - the color is taken from the CSS Table#color definition. In the attached patch it is set to:
List, Tree, Table, Text, Spinner, Combo, DateTime {
color: rgb( 39, 64, 139 );
background-color: #fff1a1;
}
Change the color here and you will see that the inheritance works.
The red color, set by the label provider is not set on the Table, but on the item itself TableItem#setForeground( index, color ). Currently, hover color (set by CSS) has a priority over the cell color (set by label provider).
Same must be with the "selected" and "selected:unfocused" state, I guess. This is more worse because you do not see the red color even if the table is unfocused. So, does "currently" mean that this might be changed or that this is the strategy that was decided to be like that? I think, it should behave like in HTML. In HTML the font or bg colors directly set on a "td" overwrite any css definition. This makes sense, because if we set a color explicitly, than we really want to be this color displayed. I attached a little html example to demonstrate this behavior with hove and seleted state. Created attachment 182960 [details]
HTML Table with an example
I'm sorry to say, but this behavior is done intentionally in all list-like widgets and I doubt, we will change it. Hover and selection colors (from CSS) have higher priority than the cell or selection colors. Well, wouldn't then be 'transparent' the best choice to do this, so that if we define the hover to be of color 'transparent' than there is no change to the color at all? Okay, first this: In our theming, "inherit" and "transparent" currently are treated as the same value, as explained in Bug 324833 and Bug 302459. For the "color" property "inherit" works as it should, while "transparent" does not. (It has the same effect as "inherit".) It is something we should fix, but it is not directly related to the issue at hand: Transparent means the text should be, as the name implies, be not visible at all. The "background-color" property should (for now) be excluded from this discussion. Then this: - Hover and selection colors from the theming always have a higher priority than the colors set programatically. - Colors that are set programatically have a higher priority than normal theming-colors. - The behavior of color-inheritance in RAP is that if a color has the value "inherit", the color that is set on the parent-widget will be used. (like HTML) - The parent of a table-item/row is the table. A table-cell, while it is themable to an extend, does technically not exist as a real widget on the client, its just a div. - The color set on a specific table-cell has a higher priority than the color of the table-item, but only because its explicitly implemented that way, not because of normal inheritance. - Since cells are not treated as children of a row, the rows hover/selection color still has a higher priority than the cell-color. So, the order of priority for text-color in Table currently is: TableItem Theming (only Hover/Select, can inherit from Table) TableItem cell-foreground TableItem foreground TableItem Theming (inherit from Table) Table foreground Table Theming We should try to come up with a solution that could solve this issue without messing with our theming too much AND still enables the current behavior if needed. I have a few ideas, but i want to think them through first. Thanks for the detailed explanation. Looking forward to a solution :-) Okay, i basically have 3 ideas, but none is ideal: 1. Tread cells as children of row/item, so they always overwrite the item-theming. (Like in HTML-tables) -> Pro: The fastet way to fix this. -> Con: Current behavior is no longer possible. 2. Using a combination of a new state (like "customcolor") and a change in the inherit-priority. -> Pro: It probably allows all needed combination. -> Con: It would create is very akward/confusing API in my opinion. 3. Introduce the "!important" (*) keyword instead of some states having a higher priority than the programatically set values. -> Pro: Works and would make the theming better reflect the actual behavior. -> Con: Using "!important" in the default-theme would make it impossible for any custom-theme to change the behavior back to what the reporter wants. It would have to be changed in the default-theme; That is, unless we don't implement that aspect of the "!important" behavior, so it only means "theming ovewrites SWT-API". Otherwise we would have the leave it out of the default-theme, then it would work fine, but it also would change the current default-behavior. (*) Here are good examples for "!important": http://www.electrictoolbox.com/using-important-css/ Personally i would prefere the third solution over any other, its simply the most elegant option and would make more sense than the current situation. On the other hand, it would also be a lot of work to implement. Since we cant fix it right away anyway, we should first carefully think about what to do about this issue. Wouldn't the !important solution only work in css files? I actually want the swt font color to overwrite any theming color of hover and selection, for example. I can think of a custom style 'swt', for example: color:swt; If this property is set to i.e. a TableItem then the strategy should be: If swt foreground color is set, then use this, otherwise treat 'swt' like as if it was not set or 'inherit'. And this should also apply to background... This is also a very tempting idea, since its very simple. But imho "!important" would be more logical than once again invent something new.
About your question:
Yes, "!important" could only be used in css. It still solves you problem:
As i explained, rules with certain states ("select", "hover") currently have a higher priority than the swt-colors. Compared to html/css, this is not very logical. The idea would be to discard this behavior completely.
Instead swt-colors would ALWAYS override css (much like an inline "style"-attribute in HTML), UNLESS the css-property has the "!important" keyword (also like HTML). This would only be done so one can still have the current behavior if needed. Otherwise, just leave "!important" out, and it would do exactly what you want - swt-api overwrites theming.
Ah ok, now I got you. This sounds good and logical. I don't think that changing the current default-behavior is a problem, as the default theme is changed in 1.4 anyway. You could then put the !importo into the classic theme with themeId org.eclipse.rap.rwt.theme.classic, I guess. With the implementation of Table-RowOverlay (Bug 364969), this issue no longer exists. Priority of colors now depends on which css element (Table-RowOverlay or TableItem) is used for selection/hover theming. |
Created attachment 182902 [details] Patch for the Demo to reproduce we have a tableviewer with some columns that have a different font color using the column label provider to show a disabled state in some cases. Now the issue we face is, that this color is overwritten by "TableItem:hover", "TableItem:selected", and "TableItem:selected:unfocused". If we set the font color to inherit or transparent (btw. is there any differnce between this?) we would expect, that the color is take from the table as defined in the column label provider. But this does not happen. It is always black. With the attached patch for your demo you can see, that some lines are displayed in red, but if you hover it then the color gets changed althoug the css file contains color: inherit; for TableItem:selected, TableItem:selected:hover. Inherit or transparten (if there is a difference) means to us, that the color should still be red as we do not want it to be overwritten.