-
Notifications
You must be signed in to change notification settings - Fork 759
[css-fonts-5] Define behavior for <meta text-scale> from #12380 #13052
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi @JoshTumath , this is my fork of Keith's initial PR. I left the definition of env(preferred-text-scale) in css-env-1 and just referred to it from here. Even though this PR is not fully complete (I didn't mention media queries, for example), I'm hoping to get it in as a kind of MVP spec. Let me know if you think something major is missing and definitely let me know if something is not correct. |
fantasai
left a comment
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.
Also, in general, please follow https://rhodesmill.org/brandon/2012/one-sentence-per-line/ :)
css-fonts-5/Overview.bs
Outdated
| The recognized keywords in the [=text-scale=] | ||
| <code class=html><meta></code> element are: | ||
|
|
||
| <ul> | ||
| <li><code class="index" lt="legacy!!text-scale-meta">legacy</code></li> | ||
| <li><code class="index" lt="scale!!text-scale-meta">scale</code></li> | ||
| </ul> |
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.
I think it would be better to follow the DL format that we have for property values: list them in the DT with a DFN tag, and give them a definition that's understandable to authors wrt what they do.
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.
Gave it a try. Let me know if you meant something different.
Thanks for pointing that out. |
JoshTumath
left a comment
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.
Hi @JoshTumath , this is my fork of Keith's initial PR. I left the definition of env(preferred-text-scale) in css-env-1 and just referred to it from here. Even though this PR is not fully complete (I didn't mention media queries, for example), I'm hoping to get it in as a kind of MVP spec. Let me know if you think something major is missing and definitely let me know if something is not correct.
This looks great! As far as I can tell, it covers everything in the Explainer.
Should the font-size section of the spec also redefine the absolute size keywords in relation to the meta tag/env var?
Once this is merged, we'll need to add the meta extension to this WHATWG wiki page: https://wiki.whatwg.org/wiki/MetaExtensions
Co-authored-by: Josh Tumath <josh.tumath@bbc.co.uk>
- Sentence about >1 meta tag - s/medium/initial/g - earlier example
|
@fantasai I'd love to get another round of feedback on this, or a stamp of approval if it's sufficient as is! |
css-fonts-5/Overview.bs
Outdated
| is recognized as setting the computed value | ||
| of the ''font-size/medium'' font size | ||
| and consequently scaling the computed size of the other <<absolute-size>> keywords. |
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.
I think instead of this you want to create a concept and say that it controls this concept.
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.
I factored the concept into "text scale factor" and refer to that. Is that approximately what you meant?
Initial definition of
<meta name="text-scale" content="...">resolved on in #12380