Community
Participate
Working Groups
I suggest we allow to style the text color and the background color of the HTML content of the tips. IIRC we do in other places by adding placeholders to the CSS and replacing them via the parsing code.
Matthias, I think you did this parsing and replacing in the past.
Created attachment 274159 [details] Screenshot of the while tips content area in the dark theme
Where can I find the code for that? The main idea is to have placeholders in the CSS. Before handing over the HTML (incl. the CSS) to the browser widget read out color preferences and replace the placeholders for "color" and "background" color with the values from the preferences. For an example: see how HTMLPrinter#insertPageProlog is used in JavaDocHover#getHoverInfo This only works as long are you do not have links to external web-pages. Because when a user clicks on these links the browser is loading the external webpage and this is displayed as is. I tried once with handling the link-clicking on my own (instead of letting the browser do it's job). But this flickers and has a lot of other issues. So this is not an option.
(In reply to Matthias Becker from comment #3) > Where can I find the code for that? > > The main idea is to have placeholders in the CSS. Before handing over the > HTML (incl. the CSS) to the browser widget read out color preferences and > replace the placeholders for "color" and "background" color with the values > from the preferences. > > For an example: see how HTMLPrinter#insertPageProlog is used in > JavaDocHover#getHoverInfo > > This only works as long are you do not have links to external web-pages. > Because when a user clicks on these links the browser is loading the > external webpage and this is displayed as is. I tried once with handling the > link-clicking on my own (instead of letting the browser do it's job). But > this flickers and has a lot of other issues. So this is not an option. Hi, the pages are here. They are external but completely ours. https://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/org.eclipse.jdt.tips.user/content https://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/org.eclipse.jdt.tips.user/content/content/855062302655209472/855062302655209472.html
(In reply to Wim Jongman from comment #4) > Hi, the pages are here. They are external but completely ours. > > https://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/ > org.eclipse.jdt.tips.user/content > > https://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/ > org.eclipse.jdt.tips.user/content/content/855062302655209472/ > 855062302655209472.html And where is the code that loads it an put's it into the browser widget?
Tips can be SWT, serve their own HTML or link to a webpage. The tip rendering code starts here [1a]. The current set of Java tips links to a page [2]. So that would cause the problems you mention in comment #3. It is also possible to include the HTML inside the tip like in this provider [3]. As you can see from [2] the framework already supports substitution variables that would support your strategy [1b]. [1a] https://git.eclipse.org/c/platform/eclipse.platform.ua.git/tree/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipComposite.java#n379 [1b] https://git.eclipse.org/c/platform/eclipse.platform.ua.git/tree/org.eclipse.tips.json/src/org/eclipse/tips/json/internal/JsonHTMLTip.java#n84 [2] https://git.eclipse.org/c/platform/eclipse.platform.common.git/tree/bundles/org.eclipse.jdt.tips.user/content/provider.json [3] https://git.eclipse.org/c/platform/eclipse.platform.ua.git/tree/org.eclipse.tips.examples/src/org/eclipse/tips/examples/json/provider.json
(In reply to Wim Jongman from comment #6) > Tips can be SWT, serve their own HTML or link to a webpage. The tip > rendering code starts here [1a]. > > The current set of Java tips links to a page [2]. So that would cause the > problems you mention in comment #3. > > It is also possible to include the HTML inside the tip like in this provider > [3]. As you can see from [2] the framework already supports substitution > variables that would support your strategy [1b]. It does not make sense if the HTML comes form the JSON or is loaded from an external site. In both cases we could "manipulate" the HTML before displaying it. For the case where the HTML comes from the JSON you already use fBrowser.setText. For the URL-Case you would also need to load the HTML on your own and also call fBrowser.setText. It breaks as soon the html contains links that link outside of the tip page (see comment 3). All your example are plain HTML without a stylesheet. The tips framework could provide a default stylesheet that is always added to the HTML.
(In reply to Matthias Becker from comment #7) > All your example are plain HTML without a stylesheet. The tips framework > could provide a default stylesheet that is always added to the HTML. +1
New Gerrit change created: https://git.eclipse.org/r/123305
I just pushed a first implementation. That implementation still needs some clean up. Details are in the commit message. I won't have time to work on this in the next weeks. So it would be good if Wim could finish this.
Thanks Matthias. Your strategy is clear. I think we need to make this more formal and create a specialization of IHtmlTip to allow style sheet injection or something simmilar. We need to check what happens if the fetched html page already contains conflicting styles?
(In reply to Wim Jongman from comment #11) > We need to check what happens if the fetched html page already contains > conflicting styles? Should we require that html pages for tip do not provide their own styling? Otherwise controlling a consistent UIX is really hard.
(In reply to Lars Vogel from comment #12) > (In reply to Wim Jongman from comment #11) > > We need to check what happens if the fetched html page already contains > > conflicting styles? > > Should we require that html pages for tip do not provide their own styling? > Otherwise controlling a consistent UIX is really hard. We can reverse my previous idea. Create a specialization of IHtmlTip that does NOT allow style injection. Also keep in mind that screenshots and movies will not be styled.
(In reply to Lars Vogel from comment #12) > (In reply to Wim Jongman from comment #11) > > We need to check what happens if the fetched html page already contains > > conflicting styles? > > Should we require that html pages for tip do not provide their own styling? > Otherwise controlling a consistent UIX is really hard. +1 for that. A rule could be that the tips should only provide a HTML fragment that then is put between <body></body> and get's a generic CSS. Another rule could be that the HMTL should not rely on background and forground color. So the tip framework could also add an additional <style>-element after the (potentionally existing <style>. As the "C" in CSS stands for "cascading" these styles would be more specific (and overrule) the already existing specifications. Note that HTMLPrinter does not add css rules but the "text" and the "bgcolor" attributes on the <body> element. As you see in https://www.w3schools.com/tags/tag_body.asp these attributes are deprecated and no longer part of HTML5. And as you see on https://www.w3schools.com/code/tryit.asp?filename=FRNK6GARW2H9 CSS rules overrule these attributes on the <body> tag (tested on chrome, safari and IE) A the tips are new I would define that rule from the beginning on. The HTML code of the above link is: <!DOCTYPE html> <html> <head> <style> body { background-color:green; color:red;} </style> </head> <body bgcolor="#FF0000" text="green"> <h1>Hello world!</h1> </body> </html>
(In reply to Wim Jongman from comment #13) > Also keep in mind that screenshots and movies will not be styled. Yes this is true. But there's no way of doing this. So when designing the tips one should check if the tip looks fine with light and dark background. That could be a recommendation of tip-authors.
(In reply to Wim Jongman from comment #13) > We can reverse my previous idea. Create a specialization of IHtmlTip that > does NOT allow style injection. I think we should ignore any provided styling in all HTML pages by default
(In reply to Matthias Becker from comment #14) > Note that HTMLPrinter does not add css rules but the "text" and the > "bgcolor" attributes on the <body> element. As you see in > https://www.w3schools.com/tags/tag_body.asp these attributes are deprecated > and no longer part of HTML5. > And as you see on > https://www.w3schools.com/code/tryit.asp?filename=FRNK6GARW2H9 CSS rules > overrule these attributes on the <body> tag (tested on chrome, safari and IE) > > A the tips are new I would define that rule from the beginning on. > > The HTML code of the above link is: > <!DOCTYPE html> > <html> > <head> > <style> > body { background-color:green; color:red;} > </style> > </head> > <body bgcolor="#FF0000" text="green"> > <h1>Hello world!</h1> > </body> > </html> I created bug 535108 for that.
(In reply to Lars Vogel from comment #16) > (In reply to Wim Jongman from comment #13) > > We can reverse my previous idea. Create a specialization of IHtmlTip that > > does NOT allow style injection. > > I think we should ignore any provided styling in all HTML pages by default We cannot remove styling just because we want to add a color. Styling is much more than colors.
(In reply to Wim Jongman from comment #18) > We cannot remove styling just because we want to add a color. Styling is > much more than colors. I agree with Wim. That why it's called "cascading". We can just add an additional stylesheet that just sets the background and foreground color of the body element. Tip-authors have to deal with the fact that their tip is displayed with different background/foreground colors.
(In reply to Matthias Becker from comment #19) > (In reply to Wim Jongman from comment #18) > > We cannot remove styling just because we want to add a color. Styling is > > much more than colors. > > I agree with Wim. That why it's called "cascading". We can just add an > additional stylesheet that just sets the background and foreground color of > the body element. > Tip-authors have to deal with the fact that their tip is displayed with > different background/foreground colors. I have to admit that I'm not familiar with how tips are transmitted, but in my opinion, tip-authors should not rely on any formatting at all. It should be all up to the mechanism presenting them. If you start fixing formatting, you might end up digging a very deep hole.
(In reply to Torkild Resheim from comment #20) > (In reply to Matthias Becker from comment #19) > > (In reply to Wim Jongman from comment #18) > > > We cannot remove styling just because we want to add a color. Styling is > > > much more than colors. > > > > I agree with Wim. That why it's called "cascading". We can just add an > > additional stylesheet that just sets the background and foreground color of > > the body element. > > Tip-authors have to deal with the fact that their tip is displayed with > > different background/foreground colors. > > I have to admit that I'm not familiar with how tips are transmitted, but in > my opinion, tip-authors should not rely on any formatting at all. It should > be all up to the mechanism presenting them. If you start fixing formatting, > you might end up digging a very deep hole. This is very limiting approach but a very stable one. We could start with that and if we then see "real" use-cases for styling by the tip author we can add it later on. Mabye this is a YAGNI (https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it)
(In reply to Matthias Becker from comment #21) > (In reply to Torkild Resheim from comment #20) > > (In reply to Matthias Becker from comment #19) > > > (In reply to Wim Jongman from comment #18) For HTML tips there are two choices. Either the HTML is inlined and fetched with getHtml() and an optional getImage() or it is fetched from an URL directly with getUrl() In the former case we have the html in our hands and we can massage it. In the latter case we need to fetch the html first instead of doing browser.setUrl(url) Getting default styles for font and background could be done with methods that tip authors could override in case they don't want it. As a sidenote, we can also inject javascript for the pages to be able to interact with the workbench.
