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

Multi-Thumb Slider: Add ability to move thumbs by clicking rail #3172

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

Conversation

jongund
Copy link
Contributor

@jongund jongund commented Nov 5, 2024

Fixes #3118

Updates the slider to:

  • Use const and let instead of var
  • Added click event on rail to move the minimum and maximum thumbs

Preview

Preview updated multi-thumb slider example in compare branch

Review checklist

Reviewers: To learn what needs to be covered by each review, Follow the link for the type of review to which you are assigned.

Not applicable:


WAI Preview Link (Last built on Tue, 05 Nov 2024 19:15:57 GMT).

@jongund jongund linked an issue Nov 5, 2024 that may be closed by this pull request
@mcking65 mcking65 changed the title Updated multi-thumb slider to support rail click Multi-Thumb Slider: Add ability to move thumbs by clicking rail Nov 12, 2024
@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 3172 - Add rail click functionality to multi-thumb slider.

The full IRC log of that discussion <jugglinmike> Topic: PR 3172 - Add rail click functionality to multi-thumb slider
<jugglinmike> github: https://github.com//pull/3172
<jugglinmike> Matt_King: This adds the functionality to the multi-thumb slider which we discussed a few weeks ago. It allows people to click on the rail to move the sliders--it moves the closest one
<jugglinmike> Matt_King: This changed JavaScript and CSS
<jugglinmike> Matt_King: It seems there's a failing check... It says the JavaScript linter is failing, jongund
<jugglinmike> Matt_King: Does anybody think we need to change any of the documentation about the example? It's a change to mouse behavior--it doesn't change keyboard. It doesn't change roles, states or properties, either.
<jugglinmike> jongund: The CSS change allows pointer events to travel through a particular element
<jugglinmike> Matt_King: So this doesn't change the visual appearance?
<jugglinmike> jongund: No. However, it occurs to me that some folks might prefer the rail itself to be a few pixels taller now that it is click-able
<jugglinmike> Matt_King: There's no change to design, so we need somebody to test the functionality, and we need someone to perform a code review
<jugglinmike> jongund: Are we supposed to be applying the "use strict" directive to all JavaScript files?
<jugglinmike> howard-e: Yes, that seems to be the case
<jugglinmike> jongund: I also updated the JavaScript to replace all the "var" declarations with "let" declarations
<jugglinmike> Matt_King: Should this be tested both with a mouse and with touch events, jongund?
<jugglinmike> jongund: I only used "click" events, but probably we should test with both
<jugglinmike> lola: I can volunteer if someone doesn't mind walking me through the changes that I'm looking for
<jugglinmike> Matt_King: The kind of testing that we want to do is documented in a link in a comment at the top of the pull request
<jugglinmike> lola: Okay, then, I can just use that
<jugglinmike> Matt_King: The specific change that jongund made is described in the linked issue. What you're looking for in this functional review is the pointer behavior is as described in the issue across multiple browsers
<jugglinmike> lola: Got it
<jugglinmike> howard-e: I can perform the code review
<jugglinmike> howard-e: jongund, you should be able to correct the formatting issues by running the command "npm run fix"
<jugglinmike> lola: My GitHub handle is lolaodelola

Copy link

@lolaodelola lolaodelola left a comment

Choose a reason for hiding this comment

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

This works as expected on the browsers I was able to test on (I currently don't own Windows or Android devices):

  • Safari on iOS and macOS
  • Firefox on macOS
  • Chrome on macOS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Horizontal Multi-Thumb Slider Example is failing SC 2.5.7
4 participants