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

feat: improvements on glossary a11y #34095

Merged
merged 11 commits into from
Jun 27, 2024
Merged

feat: improvements on glossary a11y #34095

merged 11 commits into from
Jun 27, 2024

Conversation

PassionPenguin
Copy link
Contributor

Description

  • improved links to keep consistent with other entries.

@PassionPenguin PassionPenguin requested a review from a team as a code owner June 13, 2024 03:09
@PassionPenguin PassionPenguin requested review from dipikabh and removed request for a team June 13, 2024 03:09
@github-actions github-actions bot added the Content:Glossary Glossary entries label Jun 13, 2024
@github-actions github-actions bot added the size/s [PR only] 6-50 LoC changed label Jun 13, 2024
Copy link
Contributor

github-actions bot commented Jun 13, 2024

Preview URLs

External URLs (13)

URL: /en-US/docs/Glossary/Accessibility_tree
Title: Accessibility tree


URL: /en-US/docs/Glossary/Accessibility
Title: Accessibility


URL: /en-US/docs/Glossary/Accessible_name
Title: Accessible name

(comment last updated: 2024-06-23 02:45:56)

@PassionPenguin PassionPenguin changed the title feat: improvements on a11y glossary entries feat: improvements on glossary a11y Jun 13, 2024
@github-actions github-actions bot added size/m [PR only] 51-500 LoC changed and removed size/s [PR only] 6-50 LoC changed labels Jun 13, 2024
@@ -6,7 +6,7 @@ page-type: glossary-definition

{{GlossarySidebar}}

**Accessibility** (**A11Y**) refers to best practices for keeping a website usable despite physical and technical restrictions. Web accessibility is formally defined and discussed at the {{Glossary("W3C")}} through the {{Glossary("WAI", "Web Accessibility Initiative (WAI)")}}.
**Accessibility** (often abbreviated to **A11y** — as in, "a", then 11 characters, and then "y") in web development means enabling as many people as possible to use websites, even when those people's abilities are limited in some way. Web accessibility is formally defined and discussed at the {{Glossary("W3C")}} through the {{Glossary("WAI", "Web Accessibility Initiative (WAI)")}}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fly-by comment (meaning I choose not to arbitrate on this), but the format (**A11Y**) is a very well understood way of indicating contraction/short form of a word. I find "(often abbreviated to A11y — as in, "a", then 11 characters, and then "y")" as verbose with no added value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

many entries contains these abbreviations descriptions. this above was just copied from Web/Accessibility lol.

i totally agree with your point. l18n, l10n, ... this is a pretty common abbr way in computer science. maybe we can also remove Web/Accessibility's one.

Screenshot 2024-06-14 at 11 06 02 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

I would argue that it might be a good thing to keep the verbose derivation of A11y in the Glossary for absolute beginners to web development. We can consider removing it from the Web/Accessibility page.

@github-actions github-actions bot added size/s [PR only] 6-50 LoC changed and removed size/m [PR only] 51-500 LoC changed labels Jun 14, 2024
@dipikabh
Copy link
Contributor

Hi @PassionPenguin, thanks for this PR. To better understand the motivation, could you help explain the context for these changes and the reason for adding/deleting some of the links in "See also" across the three pages?

For example, I noticed that in files/en-us/glossary/accessible_name/index.md, links to [ARIA roles](/en-US/docs/Web/Accessibility/ARIA/Roles) and [ARIA attribute](/en-US/docs/Web/Accessibility/ARIA/Attributes) have been removed, and a link to the ARIA glossary page has been added instead.

Please check our "See also section" guidelines to see the recommended "Order of links" (that is, links to MDN pages before external links). Also check the broken links in https://pr34095.content.dev.mdn.mozit.cloud/en-US/docs/Glossary/Accessibility#see_also.

Let me know if you have any questions. We're more than happy to land improvements, but providing some context will help us understand and review them better.

files/en-us/glossary/accessibility/index.md Outdated Show resolved Hide resolved
Comment on lines 19 to 20
- [The W3C Web Accessibility Initiative (WAI)](https://www.w3.org/WAI/)
- [Accessible Rich Internet Applications (WAI-ARIA)](https://w3c.github.io/aria/)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

put WAI and WAI-ARIA together, given they are in the order of

  • WAI, the organization
  • WAI-ARIA, the spec put forward by the org

@@ -18,7 +18,7 @@ There are four properties in an accessibility tree object:
- : How do we describe this thing, if we want to provide more description beyond the name? The description of a table could explain what kind of information the table contains.
- [**role**](/en-US/docs/Web/Accessibility/ARIA/Roles)
- : What kind of thing is it? For example, is it a button, a nav bar, or a list of items?
- **state**
- [**state**](/en-US/docs/Web/Accessibility/ARIA/Attributes)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added link to attributes given aria-state is under attributes page right now

@@ -22,11 +22,21 @@ To create an association between visible content and an element or multiple text

Many elements, such as sections of textual content, don't need an accessible name. All controls should have an accessible name. All images that convey information and aren't purely presentational do too.

Assistive technologies will provide the user with the accessibility name property, which is the accessible name along with the element's role. While many elements don't need an accessible name, some content [roles](/en-US/docs/Web/Accessibility/ARIA/Roles) can benefit from having an accessible name. For example, a [`tabpanel`](/en-US/docs/Web/Accessibility/ARIA/Roles/tabpanel_role) is a section of content that appears after a user activates the associated element with a [`tab`](/en-US/docs/Web/Accessibility/ARIA/Roles/tab_role) role. This role can be set on an element with no needed name, like the {{HTMLElement("div")}} element. The `tab` is the control and must have an accessible name. The `tabpanel` is the child (content section) of the `tab`. Adding `aria-labelledby` to the `tabpanel` is a best practice.
Assistive technologies will provide the user with the accessibility name property, which is the accessible name along with the element's role. While many elements don't need an accessible name, it's necessary to provide accessible name to override or to supplement the content of an element with specified [roles](/en-US/docs/Web/Accessibility/ARIA/Roles). For example, a [`tabpanel`](/en-US/docs/Web/Accessibility/ARIA/Roles/tabpanel_role) is a section of content that appears after a user activates the associated element with a [`tab`](/en-US/docs/Web/Accessibility/ARIA/Roles/tab_role) role. This role can be set on an element with no needed name, like the {{HTMLElement("div")}} element. The `tab` is the control and must have an accessible name. The `tabpanel` is the child (content section) of the `tab`. Adding `aria-labelledby` to the `tabpanel` is a best practice.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PassionPenguin and others added 2 commits June 20, 2024 09:19
Co-authored-by: Dipika Bhattacharya <[email protected]>
@PassionPenguin
Copy link
Contributor Author

Hi @PassionPenguin, thanks for this PR. To better understand the motivation, could you help explain the context for these changes and the reason for adding/deleting some of the links in "See also" across the three pages?

For example, I noticed that in files/en-us/glossary/accessible_name/index.md, links to [ARIA roles](/en-US/docs/Web/Accessibility/ARIA/Roles) and [ARIA attribute](/en-US/docs/Web/Accessibility/ARIA/Attributes) have been removed, and a link to the ARIA glossary page has been added instead.

Please check our "See also section" guidelines to see the recommended "Order of links" (that is, links to MDN pages before external links). Also check the broken links in https://pr34095.content.dev.mdn.mozit.cloud/en-US/docs/Glossary/Accessibility#see_also.

Let me know if you have any questions. We're more than happy to land improvements, but providing some context will help us understand and review them better.

Thx for your kindly reviews!

a. links are reverted (while i still consider is it necessary to open up a new page on discussing too detailed computation and recommendation provided by WAI accname and WAI-APG)
b. links order. that have been discussed before, as i made several changes to make links better, though, some reviewers thought that order of links are just recommendation but not stereotype for writer to create better articles. in this case, accname are closely related to accessible name (as that's the spec concentrating on accessible name's implementation and computation) ref
c. i have added motivations per changes! sorry for not providing them in time

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.

Hello @PassionPenguin, thanks for taking the time to explain the reason for adding the links. I was mostly looking for an overall context for the changes. Perhaps in the future, filling in the PR template questions could help.

I apologize for the mixed messages from reviewers about the order of links in "See also". I have a different opinion but will let it go.

I've left a few suggestions in the rest of the text. Thanks a lot for your contributions.

files/en-us/glossary/accessibility/index.md Outdated Show resolved Hide resolved
files/en-us/glossary/accessibility/index.md Outdated Show resolved Hide resolved
- [Web Accessibility In Mind](https://webaim.org/)
- [Glossary](/en-US/docs/Glossary)
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed a different style has been adopted here: #33423 (comment), and I understand you might have already implemented it on many pages, so I won't ask you to revert it.

However, to keep the list (and the page length) contained, I would have formatted this as (no link on Glossary):

For one term in in the list:

  • Related glossary terms: <term 1 link>

For multiple terms:

  • Related glossary terms: <term 1 link>, <term 2 link>

Copy link
Member

Choose a reason for hiding this comment

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

FWIW I would have personally gone with

  • <term 1 link>
  • <term 2 link>

Without any extra text, per see also guidelines, but I have no feelings towards any of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI, i have been translated pages under glossary folder for some time (processed about 300-350 files and PRs), and find that in most cases they are arranged in that style, and with suggestion from @bsmth, using the style of the most common case but without a link, i.e.

- Glossary
  - a.
  - b.

files/en-us/glossary/accessibility/index.md Outdated Show resolved Hide resolved
files/en-us/glossary/accessibility/index.md Outdated Show resolved Hide resolved
files/en-us/glossary/accessible_name/index.md Outdated Show resolved Hide resolved
Co-authored-by: Dipika Bhattacharya <[email protected]>
@dipikabh
Copy link
Contributor

This can be merged. Keeping it open for a short time in case we want to apply changes similar to #34165.

@bsmth
Copy link
Member

bsmth commented Jun 27, 2024

This can be merged. Keeping it open for a short time in case we want to apply changes similar to #34165.

Thanks a lot! I also think the changes look good, with one comment that I think linking to /Glossary is not necessary, so I would just prefer not to do this:

- [Glossary](/en-US/docs/Glossary)
  - {{Glossary("Term1")}}

And do this instead:

- Glossary
  - {{Glossary("Term1")}}
  - {{Glossary("Term2")}}

Or indeed

- {{Glossary("Term1")}}
- {{Glossary("Term2")}}

@dipikabh
Copy link
Contributor

dipikabh commented Jun 27, 2024

linking to /Glossary is not necessary

Agree. I think we can do a batch update for all such instances (I see around 40) in a new PR, and land this one for now.

I would prefer the following:

- Related glossary terms:
  - {{Glossary("Term1")}}
  - {{Glossary("Term2")}}

Reasons:

  • There is a colon missing at the start of the list.
  • We need to qualify "Glossary" slightly considering we're already on a glossary page.
  • It would be nice to say upfront before the list that the links are glossary terms and not guide or reference pages (so not the last suggestion).

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, @PassionPenguin

@dipikabh dipikabh merged commit ad2f2bb into mdn:main Jun 27, 2024
8 checks passed
@PassionPenguin PassionPenguin deleted the a11y branch June 27, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Glossary Glossary entries size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants