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

Add deprecation warning to Collapsible Listbox example #1852

Merged
merged 6 commits into from Aug 30, 2021
Merged

Add deprecation warning to Collapsible Listbox example #1852

merged 6 commits into from Aug 30, 2021

Conversation

smhigley
Copy link
Contributor

@smhigley smhigley commented Apr 9, 2021 •
edited by pr-preview bot

Resolves #657

Includes the following two changes:

  • Updates the link on the main aria-practices.html page to point to the select-only combobox with a text note
  • Adds a deprecation warning to the listbox example page

Preview link for example page

Preview | Diff

carmacleod
carmacleod approved these changes Apr 9, 2021
View reviewed changes
Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

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

Looks good!
Thanks, @smhigley !
Approving whether or not the little suggested change is included.

@mcking65
Copy link
Contributor

@smhigley

The deprecation warning looks good.

I think we should also remove the link to the collapsable listbox example from:

  • Scrollable Listbox Example
  • Example Listboxes with Rearrangeable Options
  • Listbox Example with Grouped Options

I'd like to discuss whether we note deprecation in the main document or remove the link. I'm leaning toward removing the link. We might also want to put something in the metadata, perhaps a data attribute on the H1, that would tell the indexing code to ignore it.

Also, what about adding "(Deprecated)" to the beginning of the page title tag and the start of the H1? Hmm, maybe that could be the indicator to the indexing script.

@mcking65 mcking65 added the agenda Include in upcoming Authoring Practices Task Force meeting label Apr 26, 2021
@a11ydoer
Copy link
Contributor

a11ydoer commented May 4, 2021

APG groupd made a decision to remove deprecated examples on May 4, 2021

@a11ydoer
Copy link
Contributor

a11ydoer commented May 4, 2021 •
edited

@jongund will update APG example index page after @mcking65 merges this issue.

@mcking65
Copy link
Contributor

mcking65 commented May 6, 2021

Note to self:

  1. Prepend (Deprecated) to title tag and H1
    1. Remove link from main doc and other example pages

@jnurthen jnurthen mentioned this pull request May 27, 2021
Closed
@jongund
Copy link
Contributor

jongund commented Jun 29, 2021

@smhigley @mcking65 @a11ydoer
The only review this pull request needs is editorial on the deprecation warning and testing if the updated link works. This should only require two maybe three people. The list of reviewers seems to accessibility testing on all of our platforms. I suggest updated the list of reviewers on this pull request.

@jongund jongund self-requested a review June 29, 2021 19:17
jongund
jongund requested changes Jun 29, 2021
View reviewed changes
Copy link
Contributor

@jongund jongund left a comment •
edited

Choose a reason for hiding this comment

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

@smhigley
I think this pull request should just delete the listbox-collapsible.html reference in aria-practices.html and not add the new reference to listbox-select-only.html.

The pull request that creates listbox-select-only.html should make it's own changes to aria-practices.html and listbox-collapsible.html so the links actually work, otherwise as a reviewer it is confusing.

@smhigley
Copy link
Contributor Author

smhigley commented Jul 1, 2021 •
edited

@jongund the added link is for the already-existing combobox-select-only example, and the links should work (or at least, they work fine for me in the preview). If people think it'd be better not to have any link there at all, I'm happy to remove it. I do think it makes sense to use this PR to decide what example links to include in the Listbox section.

@jongund
Copy link
Contributor

jongund commented Jul 1, 2021

@smhigley
The link was not working yesterday when I tested, it works today.

mcking65
mcking65 reviewed Aug 30, 2021
View reviewed changes
examples/listbox/listbox-grouped.html Outdated
@@ -2,7 +2,7 @@
<html lang="en">
<head>
<meta charset="utf-8" />
<title>Listbox Example with Grouped Options | WAI-ARIA Authoring Practices 1.2</title>
<title>(Deprecated) Listbox Example with Grouped Options | WAI-ARIA Authoring Practices 1.2</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, right change in wrong file :) I'll push a fix for this.

mcking65
mcking65 reviewed Aug 30, 2021
View reviewed changes
examples/listbox/listbox-grouped.html Outdated
@@ -26,7 +26,7 @@
</ul>
</nav>
<main>
<h1>Listbox Example with Grouped Options</h1>
<h1>(Deprecated) Listbox Example with Grouped Options</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll fix this one too

mcking65
mcking65 approved these changes Aug 30, 2021
View reviewed changes
Copy link
Contributor

@mcking65 mcking65 left a comment

Choose a reason for hiding this comment

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

OK, Looks good now. Thank you @smhigley!

@mcking65 mcking65 merged commit f4ddc33 into main Aug 30, 2021
@mcking65 mcking65 deleted the remove-listbox branch August 30, 2021 07:18
@nschonni nschonni mentioned this pull request Sep 8, 2021
Merged
@mcking65 mcking65 mentioned this pull request Oct 17, 2021
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jongund jongund jongund approved these changes

@carmacleod carmacleod carmacleod approved these changes

@mcking65 mcking65 mcking65 approved these changes

Assignees
No one assigned
Labels
agenda Include in upcoming Authoring Practices Task Force meeting
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Listbox: How to make a collapsible listbox required?
5 participants
@smhigley @mcking65 @a11ydoer @jongund @carmacleod