Skip to content
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

[css-pseudo] clarify paired cascade (#6386 + #6779 + #6774 + #7837) #6665

Merged
merged 32 commits into from
Oct 6, 2022

Conversation

delan
Copy link
Contributor

@delan delan commented Sep 22, 2021

This patch makes edits for #6386:

  • clarifies that “use” means used value (q1)
  • clarifies that the highlight colors are ‘color’ + ‘background-color’ only (q2)
    • also clarifies that ‘stroke-color’ and ‘fill-color’ are not highlight colors
  • changes the pseudos affected by paired cascade to all highlight pseudos (q3)
  • clarifies that only highlight colors disable UA defaults (q4)
  • clarifies that only the author origin disables UA defaults (q5)
    • also eliminates the old usage of “specified” in favour of a definition that doesn’t overload any terminology
  • clarifies that ‘revert’ disables UA defaults iff the result after reverting still has values in the origins above (q6a)
    • also clarifies that ‘revert-layer’ similarly depends on the origin after rolling back the cascade
  • clarifies that ‘unset’ (and other explicit defaults) disable UA defaults (q6b)
  • clarifies that ‘unset’ means ‘inherit’ for all properties in highlight styles (q7)
  • clarifies that ‘inherit’ also inherits parent’s highlight styles (@Loirooriol’s comment)
  • clarifies that UAs are still allowed to tweak highlight colors (@delan’s comment)

This patch also makes edits for #6779:

  • changes the effective default ‘color’ of ::{spelling,grammar}-error + ::highlight from ‘initial’ to ‘currentColor’ (p1)
  • fixes the contradiction between Highlight API #default-styles and highlight inheritance (p2)

This patch also makes edits for #6774 (@delan’s comment):

  • defines the inherited value of ‘color’ in the root’s highlight pseudos as ‘currentColor’ (option 2)

@delan delan changed the title [css-pseudo] clarify paired cascade (#6386) [css-pseudo-4] clarify paired cascade (#6386) Sep 22, 2021
@delan delan requested review from fantasai and frivoal September 23, 2021 12:43
@delan delan marked this pull request as ready for review September 23, 2021 12:43
@delan delan changed the title [css-pseudo-4] clarify paired cascade (#6386) [css-pseudo-4] clarify paired cascade (#6386 + #6150) Oct 5, 2021
@delan delan changed the title [css-pseudo-4] clarify paired cascade (#6386 + #6150) [css-pseudo] clarify paired cascade (#6386 + #6150) Nov 2, 2021
@delan delan added the css-pseudo-4 Current Work label Nov 2, 2021
@delan delan assigned astearns and therealglazou and unassigned astearns and therealglazou Nov 2, 2021
@delan delan changed the title [css-pseudo] clarify paired cascade (#6386 + #6150) [css-pseudo] clarify paired cascade (closes #6386) Nov 2, 2021
@delan delan changed the title [css-pseudo] clarify paired cascade (closes #6386) [css-pseudo] clarify paired cascade (closes #6386 + #6779) Nov 18, 2021
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
@delan delan changed the title [css-pseudo] clarify paired cascade (closes #6386 + #6779) [css-pseudo] clarify paired cascade (#6386 + #6779 + #6774) Apr 15, 2022
@delan
Copy link
Contributor Author

delan commented Apr 15, 2022

@fantasai, @frivoal, some notable improvements to this patch since your last reviews:

  • i’ve simplified the origins that affect paired cascade to author only (bfc6498)
  • i’ve removed the text allowing UA tweaks to author highlight colors (0c46d9d)
  • i’ve defined the inherited value of ‘color’ in the root as ‘currentColor’ (ce77850)
  • i’ve tied the default highlight colors for ::selection + ::target-text to system colors (4eb95b0)
  • i’ve rewritten and heavily simplified the new section about paired cascade itself (86bb487)

I believe the new proposed text is clearer than ever, and conveys all of the details I wanted to convey (see pr description for the complete list) in far fewer words. Please take a look!

@delan delan requested review from frivoal and fantasai April 15, 2022 08:24
@delan
Copy link
Contributor Author

delan commented Jun 30, 2022

@fantasai, @frivoal, #6774 is now resolved, so these changes are ready for review. It looks like fantasai last reviewed 44aa6ce while Florian last reviewed 24df479, in case it helps you pick up where you left off and see what has changed.

css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
@delan delan requested a review from fantasai September 21, 2022 11:39
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
css-pseudo-4/Overview.bs Outdated Show resolved Hide resolved
@delan delan changed the title [css-pseudo] clarify paired cascade (#6386 + #6779 + #6774) [css-pseudo] clarify paired cascade (#6386 + #6779 + #6774 + #7837) Oct 6, 2022
@delan delan requested a review from fantasai October 6, 2022 09:27
@fantasai
Copy link
Collaborator

fantasai commented Oct 6, 2022

Ok, looks good. Please SQUASH and rebase before merging. :)

@delan delan merged commit 4acdbd8 into w3c:main Oct 6, 2022
@delan delan deleted the clarify-paired-cascade branch October 6, 2022 18:13
@delan
Copy link
Contributor Author

delan commented Oct 6, 2022

Thank you both for all of your feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-pseudo-4 Current Work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants