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

editorial: revise custom stylesheets #2410

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

editorial: revise custom stylesheets #2410

wants to merge 31 commits into from

Conversation

pkra
Copy link
Member

@pkra pkra commented Jan 9, 2025

Resolves #2226


Preview | Diff

@pkra pkra added editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo Agenda-Editors labels Jan 9, 2025
Copy link

netlify bot commented Jan 9, 2025

Deploy Preview for wai-aria ready!

Name Link
🔨 Latest commit c9185c2
🔍 Latest deploy log https://app.netlify.com/sites/wai-aria/deploys/67af071d5ec7ad00081cab73
😎 Deploy Preview https://deploy-preview-2410--wai-aria.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pkra
Copy link
Member Author

pkra commented Jan 9, 2025

notes to self:

@pkra
Copy link
Member Author

pkra commented Jan 9, 2025

It turns out, core-aam only sets styling that are already set in common.css.

@pkra
Copy link
Member Author

pkra commented Jan 10, 2025

@jnurthen @spectranaut @daniel-montalvo I think I actually want to suggest to remove all of common.css.

Almost all of it is minor stylistic differences to what respec does (i.e., everything after the table styles).

For tables, I would suggest to switch to respec's table.def styles. Very different but that's what they're for and it seems better to stick with respec for maintenance.

The only other bit are the nested lists styles. However, only accname actually needs these (and I feel like respec or W3C TR styles should be doing a better job here - but they probably don't want to risk it but perhaps be open to opt-in classes).

[For completeness: there are 2 nested lists in aria in 4.3.2 - all other nested lists in specs are from respec TOCs where the styles are set by respec.]

I'll prep something for the next editors meeting so we can go through them together.

@pkra
Copy link
Member Author

pkra commented Jan 23, 2025

Discussed in editors' call https://www.w3.org/2025/01/22-aria-editors-minutes.html#1b79

tl;dr I will continue this PR to effectively remove common.css

@pkra
Copy link
Member Author

pkra commented Jan 23, 2025

@jnurthen looking through https://w3c.github.io/tr-design/src/README I see 1-2 other table styles: .data and .index (which is like .data but more cramped). They look more like our current tables, but have noticeably more whitespace.

Are there others you had in mind?

@jnurthen
Copy link
Member

Are there others you had in mind?

I think I was thinking about the default style without data for some of the tables at the end of the doc

pkra added a commit that referenced this pull request Feb 12, 2025
Removes styles that match  1 list in aria, accname, core-aam, pdf-aam;
only 1 minor  visual change in accname, otherwise margin-collapse helps.

Part of #2410
pkra added a commit that referenced this pull request Feb 12, 2025
All props are overruled by respec.

Part of #2410
@pkra pkra mentioned this pull request Feb 12, 2025
4 tasks
pkra added a commit that referenced this pull request Feb 12, 2025
As per editors discussion, we switch to the default respec style.

Part of #2410
pkra added a commit that referenced this pull request Feb 12, 2025
As per editors discussion, we switch to the default respec style.
Cf. #2433 for follow up on aria.js (which generates these elements).

Part of #2410
pkra added a commit that referenced this pull request Feb 12, 2025
As per editors discussion, we switch to the default respec style.

Part of #2410
pkra added a commit that referenced this pull request Feb 12, 2025
As per editors discussion, we switch to default respec / highlightjs
styles.

Part of #2410
pkra added a commit that referenced this pull request Feb 12, 2025
As per editors discussion, we remove the nested list and add
a style block with custom classes to accname.
- common.css:  remove nested OL styles
- accname/index.html
  - add style block with custom classes
  - add appropriate classes to nested lists in 4.3.2
    (except in the first note and example 3)

Part of #2410
pkra added a commit that referenced this pull request Feb 12, 2025
As per editors discussion, removes common.css and
adds respec classes to tables.
- switches existing classes to .def
- adds .def to IDL correspondance table
- switches 2 classless tables to "data"

Part of #2410
pkra added a commit that referenced this pull request Feb 12, 2025
This file no longer appears to be used anywhere.

Part of #2410
pkra added a commit that referenced this pull request Feb 12, 2025
This file no longer appears to be used anywhere.

Part of #2410
@pkra pkra mentioned this pull request Feb 12, 2025
@pkra
Copy link
Member Author

pkra commented Feb 12, 2025

As per today's call, I will switch the other specs to using respec table classes.

Then the editors can review each spec.

@MelSumner could you take a look at the nested list classes I've added to accname 42082b0? @spectranaut suggested it might be better to design the lists in a way so that new nested lists can be added without having to remember the right class. It seems prudent to ask a spec editor to do that 😄

pkra added 17 commits February 13, 2025 17:28
As per editors discussion, we remove the nested list and add
a style block with custom classes to accname.
- common.css:  remove nested OL styles
- accname/index.html
  - add style block with custom classes
  - add appropriate classes to nested lists in 4.3.2
    (except in the first note and example 3)

Part of #2410
As per editors discussion, removes common.css and
adds respec classes to tables.
- switches existing classes to .def
- adds .def to IDL correspondance table
- switches 2 classless tables to "data"

Part of #2410
This file no longer appears to be used anywhere.

Part of #2410
This file no longer appears to be used anywhere.

Part of #2410
Removes common.css and adds respec class "data" to the table
(in "4.2 Description Computation").

Part of #2410
As per editors discussion, removes common.css and
replaces .role-features with respec's .def

Part of #2410
As per editors discussion, removes common.css and
replaces .role-features with respec's .def

Part of #2410
Removes common.css and  mapping-tables.css
and adds respec's .data to all tables

Part of #2410
Removes common.css and  mapping-tables.css
and adds respec's .data to all tables

Part of #2410
Removes common.css and  mapping-tables.css
and adds respec's .data to all tables

Part of #2410
Removes common.css and  mapping-tables.css
and adds respec's .data to all tables

Part of #2410
Removes common.css and  mapping-tables.css
and adds respec's .data to all tables

Part of #2410
Removes common.css and  mapping-tables.css
and adds respec's .data to all tables

Part of #2410
Removes common.css and  mapping-tables.css
and adds respec's .data to all tables

Part of #2410
As per editors discussion, removes common.css and
replaces table classes with respec's .def

Part of #2410
At long last, it is longer in use.

Part of #2410
At long last, it is longer in use.

Part of #2410
@pkra pkra marked this pull request as ready for review February 13, 2025 16:35
@pkra
Copy link
Member Author

pkra commented Feb 13, 2025

This is ready for review.

While rebasing I fixed a typo in html-aam (extraneous closing parenthesis in the img alt section).

Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one request about the CSS for nested lists, I'd like @MelSumner to weigh in as she is the visual editor of that spec :)

Also -- I think we should have, perhaps a follow up issue, to make all the data tables in the aams use class=def to get the blue definition tables: https://www.w3.org/StyleSheets/TR/2021/README#def

<style>
.ol--upperAlpha{ list-style:upper-alpha; } /* replaces: ol ol */
.ol--lowerRoman { list-style:lower-roman; } /* replaces: ol ol ol */
.ol--lowerAlpha { list-style:lower-alpha; } /* replaces: ol ol ol ol */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we do something like putting the class "computation_algorithm" on the rool ol and then do:

.computation_algorithm ol ol { list-style:upper-alpha;  }
.computation_algorithm ol ol ol { list-style:lower-roman; }
.computation_algorithm ol ol ol ol {l ist-style:lower-alpha; }

And remove those custom classes on the nest ols, so no one has to worry about adding then when they make a nested list in a computation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor notes:

  • if .computation_algorithm goes on an OL, this needs to drop one ol from each line.
  • then these styles would still affect the two lists in the Note and Example

As I said on the call, I have no strong feelings either way. But we can also work this out later. (I'm a little worried about merge conflicts as I'm touching all the specs.)

@pkra
Copy link
Member Author

pkra commented Feb 13, 2025

Also -- I think we should have, perhaps a follow up issue, to make all the data tables in the aams use class=def to get the blue definition tables: https://www.w3.org/StyleSheets/TR/2021/README#def

@spectranaut ugh, sorry, that's my fault. I misread #2229 and thought you wanted the .data style.

I'll update the PR tomorrow.

Properly address #2229 this time.

Part of #2410
@pkra
Copy link
Member Author

pkra commented Feb 14, 2025

Also -- I think we should have, perhaps a follow up issue, to make all the data tables in the aams use class=def to get the blue definition tables: https://www.w3.org/StyleSheets/TR/2021/README#def

@spectranaut ugh, sorry, that's my fault. I misread #2229 and thought you wanted the .data style.

I'll update the PR tomorrow.

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

remove spec specific css or inline it
3 participants