-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update figure style of triple terms #168
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odd that PR Preview didn't even try :(. I used https://raw.githack.com/w3c/rdf-concepts/fea72262ea9b67b10b21402cc8ec4e5a17a96151/spec/index.html to see the rendered version.
I note that the IRI section (class="hljs abnf"
) is rendered with a white background, as are the SVGs (on Safari, it seems to render okay in Firefox). Also, the Skolem IRI example in Replacing Blank Nodes with IRIs (class="hljs-attr"
) has a white background.
Currently, But the SVGs should work... Does OS/browser-controlled work for you @gkellogg? (You may have to reload after toggling; alas, perhaps due to the JS fiddling with default behaviour.) Please also see if the SVGs themselves (e.g. this) works when toggling. I'll check on a Mac soon. |
Everything else works properly in Safari, but there could be some features used for this that CanIUse might indicate Safari doesn't yet support. I wouldn't hold up merging because of this. I'll approve, as it's definitely an improvement on the base. |
@@ -43,7 +44,10 @@ | |||
}; | |||
</script> | |||
<style> | |||
body:has(input[type='radio'][value='light']:checked) { color-scheme: light } | |||
body:has(input[type='radio'][value='dark']:checked) { color-scheme: dark } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside: I guess this is something that should be part of tr-design stylesheet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I think this solves the JS-dependency problem you noted, but is still sensitive to structural ReSpec changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. But if it's ship in tr-design alongside the definition of the selector then I guess it's fine.
This includes: - Use dedicated arrowheads to compensate for WebKit lacking context-stroke support. - Use object element instead of img, so that Webkit makes use of prefers-color-scheme for the SVG images. - With object, one external CSS can be used for all SVG figures. - Improve some naming in the CSS. - Adjust figure styles to support object elements wrapped in anchor links, and to set a relative em max-width instead of percent width.
I updated this to better support WebKit (for one, the arrow-heads didn't render before). See githack preview. @gkellogg, OS/browser-based light/dark mode should now work also for the SVGs in Safari (I've tested with Gnome Web). Getting manual color-scheme override to work through contextual color-scheme handling doesn't work yet in Safari, but that will hopefully be addressed. |
I try to view things like this through the WebDeveloper extension's Responsive Layouts, which opens iframes with a user configurable set of viewport sizes. My primary browser is Chrome (a little old Version 116.0.5845.187 (Official Build) (x86_64), because I'm stuck on a little old macOS Mojave 10.14.6 (18G9323)). ReSpec chokes when viewing the multiple viewports through githack. They seem to work OK through PR-Preview. I do notice that the font gray of comments in the EBNF does not sufficiently contrast the background gray in dark mode. This contrast is fine in light mode. Overall, things look pretty good in both light and dark mode. Is there any place or viewport size in particular you'd like me to look at, @niklasl? |
@TallTed Thanks for doing this responsive testing! (Oddly, for me, the githack version works better; and is the only one showing the SVGs.) The contrast issues are part of a respec issue relating to highlight-js (see also my comment above). We could refrain from enabling dark mode until this is resolved in respec; or merge this and open another issue to track that. My hunch is that enabling dark mode early on increases our chances of catching problems (just as we did above with the SVGs not working in WebKit). |
A quick Lighthouse run shows that (for a11y) I need to to use |
Major changes:
Minor changes:
The textual descriptions aim to align with recent updates clarifying triples-as-terms and propositions. The visual style might prompt further debate; hence I wanted to make sure dark mode is taken into account if we decide to do further changes.
Preview | Diff