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.

Sign up for GitHub

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

Jump to bottom

Move and rename "Accessibility concerns" section in CSS page templates #34984

Merged
merged 2 commits into from
Jul 31, 2024
Merged

Move and rename "Accessibility concerns" section in CSS page templates #34984

merged 2 commits into from
Jul 31, 2024

Conversation

mcaskill
Copy link
Contributor

@mcaskill mcaskill commented Jul 23, 2024 •
edited
Loading

Description

Renames "Accessibility concerns" to "Accessibility" and moves it above "Examples". In line with this discussion: github.com/orgs/mdn/discussions/430

Also removed the "optional" notice from the start of the section, as it's already clear that the section should be removed if no guidance is needed.

Motivation

Make accessibility guidance more prominent.

Additional details

Full discussion: github.com/orgs/mdn/discussions/430

Related issues and pull requests

@mcaskill mcaskill requested a review from a team as a code owner July 23, 2024 01:08
@mcaskill mcaskill requested review from hamishwillee and removed request for a team July 23, 2024 01:08
@github-actions github-actions bot added Content:Meta Content in the meta docs size/s [PR only] 6-50 LoC changed labels Jul 23, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 23, 2024 •
edited
Loading

Preview URLs

(comment last updated: 2024-07-30 20:27:43)

@mcaskill mcaskill mentioned this pull request Jul 23, 2024
Merged
@mcaskill mcaskill force-pushed the move-a11y-in-css-page-templates branch from cf35f73 to 21c180c Compare July 23, 2024 15:36
@dipikabh
Copy link
Contributor

Leaving a drive-by comment to offer another perspective: While changing the name and position of the section is okay, I believe the current wording looks perfect and presents the intent well:
"This is an optional section. You can include any warnings here for accessibility concerns that developers should be aware of while using this property. You can also include workarounds for these accessibility concerns if there are any."

In fact, because we are dropping "concerns" from the section title, we should update that bit to state other aspects this section can cover, along these lines:
"This is an optional section. Include accessibility guidelines, best practices, and potential concerns that developers should be aware of while using this property. You can also include workarounds or solutions where applicable."

I understand the inspiration for the text update might be coming from #33628, but it might be worth reviewing that change as well.

mcaskill added a commit to mcaskill/mdn-content that referenced this pull request Jul 23, 2024
@mcaskill
Update description of Accessibility section in CSS page templates
beb7caf 
As per: mdn#34984 (comment)
@mcaskill
Copy link
Contributor Author

@dipikabh I like your suggestion. It's a much better description to accompany the main change of this pull request.

@dipikabh
Copy link
Contributor

Hi @mcaskill, I'm glad you like the suggestion. In that case, please feel free to update this PR when you get the chance.

@dipikabh dipikabh self-requested a review July 30, 2024 17:23
@mcaskill
Move and rename accessibility section
4c0378e 
Changes:
- Adjust section description for CSS Function and Property templates to match Selector template.
- Remove "optional" notice in favour of closing notice.
@mcaskill
Update description of Accessibility section in CSS page templates
54689bb 
As per: mdn#34984 (comment)
@mcaskill mcaskill force-pushed the move-a11y-in-css-page-templates branch from fae9cb6 to 54689bb Compare July 30, 2024 20:26
@mcaskill
Copy link
Contributor Author

[…] In that case, please feel free to update this PR when you get the chance.

@dipikabh I had already updated the PR with your suggestion as a second commit (54689bb).

@dipikabh
Copy link
Contributor

Oh nice, sorry I missed to notice the updated text.

dipikabh
dipikabh approved these changes Jul 31, 2024
View reviewed changes
Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution, @mcaskill 🙌

@dipikabh dipikabh merged commit e3dd5fe into mdn:main Jul 31, 2024
8 checks passed
@mcaskill mcaskill deleted the move-a11y-in-css-page-templates branch July 31, 2024 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dipikabh dipikabh dipikabh approved these changes

@hamishwillee hamishwillee Awaiting requested review from hamishwillee hamishwillee is a code owner automatically assigned from mdn/yari-content-mdn

Assignees
No one assigned
Labels
Content:Meta Content in the meta docs size/s [PR only] 6-50 LoC changed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mcaskill @dipikabh