-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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 BCD keys for SVG attributes #32366
Conversation
Preview URLs (78 pages)
Flaws (100)URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
URL:
(comment last updated: 2024-03-01 04:16:32) |
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'm not really able to check this all in detail, but the pages I've looked at LGTM; nice work @queengooborg.
Thank you, @chrisdavidmills! Would you also be down to review/approve the BCD PR by chance? |
I am not, no. I can do, if no one grabs it. |
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 made a few comments. This PR is too big! Would it be possible to break it into 3:
- the updating of browser compat in the front matter
- removal of global attributes (while ensuring the links to those under "most notably" are included in their relevant pages
- removal of the attribute category landing pages
I spot checked for #1, and think it's ok.
Changes are required for #2.
I did not look at category 3.
I can try splitting it into individual PRs, but I'm afraid there's a lot of crossover between the changes and they must be merged in order to prevent issues. I'd probably recommend reviewing this PR per commit instead of all at once? |
Review #1: assuming BCD was updated, there are no visible issues here. |
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.
Review part 2: Remove landing pages for attribute categories
A few minor issues in this section.
I'll review part 3 after the pages are updated to ensure the important attributes are not omitted from the element pages.
This PR is now ready for re-review and merging, along with the BCD PR! |
The BCD PR has been approved and merged, so this will be merged once the next BCD release is out! |
This PR performs a number of updates to the SVG attributes pages to clean up the pages:
Note: review and merge along with mdn/browser-compat-data#22267.
Follow-up: remove all use of the
SVGAttr
macro.