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(slider): add range mode accessibility support #332

Merged
merged 6 commits into from
Jun 2, 2022

Conversation

wsuwt
Copy link
Collaborator

@wsuwt wsuwt commented May 25, 2022

Description

  • Add Arrow keys navigation support in range mode
  • Add Home and End support

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have checked my code and corrected any misspellings

@wsuwt wsuwt self-assigned this May 25, 2022
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented May 25, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 70e624b
Status: ✅  Deploy successful!
Preview URL: https://564497f0.refinitiv-ui.pages.dev

View logs

@wsuwt wsuwt marked this pull request as ready for review May 26, 2022 09:34
@wsuwt wsuwt added the accessibility Accessibility improvement label May 26, 2022
@@ -598,18 +598,27 @@ export class Slider extends ControlElement {
return;
}

this.changedThumb = event.target as HTMLDivElement;
const thumbName = this.changedThumb.getAttribute('name') as SliderDataName;
Copy link
Contributor

@goremikins goremikins May 31, 2022

Choose a reason for hiding this comment

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

Can you detect the thumb name in different name, e.g. if you hold the reference to thumb:

private getThumbName (target: EventTarget): SliderDataName | null {
  if (target === this.thumbFromRef.value) {
     return SliderDataName.from;
  }
  // return ....
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added ref to thumbs and refactor a bit on thumb related logics.

* @param type from or to
* @returns validated from or to
*/
private validateRange (value: string, type: SliderDataName): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this function needs better comment. I am not sure what does it try to validate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was a wrapper function of validateFrom and validateTo. I've decided to remove it and improve code comment.

@sonarcloud
Copy link

sonarcloud bot commented Jun 1, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@wsuwt wsuwt requested a review from goremikins June 1, 2022 08:37
@wsuwt wsuwt merged commit 51a2291 into feat/slider-a11y Jun 2, 2022
@wsuwt wsuwt deleted the feat/slider-range branch June 2, 2022 03:30
wsuwt added a commit that referenced this pull request Jun 6, 2022
)

* chore: update package-lock

* feat(slider): add arrow key support and add aria attributes

* test: refactor test structure and add test for arrow key interactions

* refactor: update variable names and fix typo

* feat: add focus outline style to thumb

* feat: remove input focus and focus thumb on drag

* feat: simplify `validateNumber` util function

* test: update snapshot

* fix: detect keyboard focus state from host

* fix: use tabIndex=1 instead

* feat: introduce `active` attribute for thumb

* feat: add logic to adds/removes `active` thumb focus style and add generic labels to slider and input

* test: update snapshots

* refactor: reuse `calculatePercentage` function

* fix: add min more than max case

* fix: remove invalid number checking on `calculatePercentage`

* refactor: improve variable name

* fix: add disabled back to number-field

* refactor: replace the term `percentageValue` with thumbPosition for consistency

* refactor: replace 5,10 z-index with 3,4 respectively and add comments why

* refactor: remove `validateNumber`

* fix: remove input label

* test: extract private getters to a function and remove describe layer

* refactor: rename `calculatePercentage` to `calculatePosition`

* refactor: thumb logic and use `@state`

* feat: ignore special characters onKeyDown

* refactor: remove percentage reference and initialise

* fix: add `defaultPrevented` onkeydown

* feat(slider): add `range` mode accessibility support (#332)

* feat(slider): add `range` mode accessibility support

* feat(slider): hide number-field inputs from tab sequence and screen reader

* fix: switch Home and End logic

* docs: remove deupllicated demo

* refactor: improve code readability

* refactor: keeping thumb as refs

Co-authored-by: wsuwt <[email protected]>

* docs(slider): add slider a11y docs (#341)

* feat(slider): add `range` mode accessibility support

* feat(slider): hide number-field inputs from tab sequence and screen reader

* fix: switch Home and End logic

* docs: remove deupllicated demo

* docs(slider): add slider a11y docs

Co-authored-by: wsuwt <[email protected]>

Co-authored-by: wsuwt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Accessibility improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants