Skip to content

useColorSlider should increment/decrement by channel pageSize using PageUp/PageDown #2664

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
Closed
majornista opened this issue Dec 8, 2021 · 0 comments · Fixed by #2666
Closed

useColorSlider should increment/decrement by channel pageSize using PageUp/PageDown #2664

majornista opened this issue Dec 8, 2021 · 0 comments · Fixed by #2666

Comments

@majornista
Copy link
Collaborator

🐛 Bug Report

useColorSlider PageUp/PageDown keys increment or decrement by (max-min) / 10, which is the default behavior for a native input. However, color channels have a pageSize that differs depending on the channel. For example, for RGB values the pageSize is 16, a factor of 256, and for Hue, the pageSize is 15, a factor of 360.

  1. Open https://reactspectrum.blob.core.windows.net/reactspectrum/a98461602538bcf63009a17e31900726819a4af6/storybook/index.html?path=/story/colorslider--default
  2. Navigate to the ColorSlider for the Red channel.
  3. Press Home key to set value to 0.
  4. Press PageUp to increment by pageSize.

We also need to ensure that both onChange and onChangeEnd fire with PageUp/PageDown/Home/End

🤔 Expected Behavior

ColorSlider should increment by the pageSize, 16.

😯 Current Behavior

Instead it navigates by 255 / 10, which rounds to 26.

💁 Possible Solution

See #2661 and 4ce326f

🔦 Context

Related to #2570.
Depends on #2661

💻 Code Sample

🌍 Your Environment

Software Version(s)
react-spectrum @react-aria/[email protected]
Browser Chrome version 96.0.4664.55 (Official Build) (x86_64)
Operating System macOS Big Sur version 11.6.1 (20G224)

🧢 Your Company/Team

Adobe/Accessibility

🕷 Tracking Issue (optional)
majornista added a commit that referenced this issue Dec 9, 2021
@majornista
fix(#2664): useColorSlider: refine PageUp/PageDown behaviors
61a13d3 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
@majornista majornista mentioned this issue Dec 9, 2021
Merged
5 tasks
majornista added a commit that referenced this issue Dec 9, 2021
@majornista
fix(#2664): useColorSlider: refine PageUp/PageDown behaviors
8e9b59b 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.

@trivial fix lint error
majornista added a commit to majornista/react-spectrum-v3 that referenced this issue Dec 9, 2021
@majornista
fix(adobe#2664): useColorSlider: refine PageUp/PageDown behaviors
0e52041 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
majornista added a commit that referenced this issue Dec 9, 2021
@majornista
fix(#2664): useColorSlider: refine PageUp/PageDown behaviors
8573563 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
majornista added a commit to majornista/react-spectrum-v3 that referenced this issue Dec 9, 2021
@majornista
fix(adobe#2664): useColorSlider: refine PageUp/PageDown behaviors
8b8ea3a 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
@LFDanLu LFDanLu linked a pull request Dec 15, 2021 that will close this issue
fix(#2664): useColorSlider: refine PageUp/PageDown behaviors #2666
Merged
5 tasks
majornista added a commit that referenced this issue Jan 26, 2022
@majornista
fix(#2664): useColorSlider add onChangeEnd to ColorSlider and ColorWh…
13161ca 
…eel stories
majornista added a commit that referenced this issue Jan 26, 2022
@majornista
fix(#2664): add tests for Home/PageUp at max and End/PageDown at min
312a2e4 
Per https://github.com/adobe/react-spectrum/pull/2666/files#r792201596, verify that Home/PageDown at 0 and End/PageUp at 255 won't cause onChange and onChangeEnd to be called.
majornista added a commit to majornista/react-spectrum-v3 that referenced this issue Feb 14, 2022
@majornista
fix(adobe#2664): useColorSlider: refine PageUp/PageDown behaviors
935d7ea 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
majornista added a commit to majornista/react-spectrum-v3 that referenced this issue Feb 18, 2022
@majornista
fix(adobe#2664): useColorSlider: refine PageUp/PageDown behaviors
3046fd4 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
majornista added a commit to majornista/react-spectrum-v3 that referenced this issue Feb 18, 2022
@majornista
fix(adobe#2664): useColorSlider: refine PageUp/PageDown behaviors
1b93321 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
majornista added a commit to majornista/react-spectrum-v3 that referenced this issue Feb 24, 2022
@majornista
fix(adobe#2664): useColorSlider: refine PageUp/PageDown behaviors
3ecc458 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
majornista added a commit that referenced this issue Feb 24, 2022
@majornista
fix(#2664): useColorSlider: refine PageUp/PageDown behaviors
8256492 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
majornista added a commit that referenced this issue Feb 28, 2022
@majornista
fix(#2664): useColorSlider: simplify per code review
c61bbb6
majornista added a commit that referenced this issue Feb 28, 2022
@majornista
fix(#2664): useColorSlider: refine PageUp/PageDown behaviors
463837d 
- Omit change events when value doesn't change.
- Refine snapValueToStep for incrementing or decrementing from max.
majornista added a commit that referenced this issue Feb 28, 2022
@majornista
fix(#2664): useColorSlider: simplify per code review
f533056
majornista added a commit that referenced this issue Mar 1, 2022
@majornista
fix(#2664): useColorSlider: fix increment/decrementThumb default step…
60606e3 
…Size

If the channel range step is less than 1, like 0.01, and stepSize is not passed as a prop, the slider will always increment or decrement by 1.
majornista added a commit that referenced this issue Mar 3, 2022
@majornista
fix(#2664): remove public API for step and pageStep from ColorSlider …
6bd92e0 
…and ColorWheel
majornista added a commit that referenced this issue Mar 3, 2022
@majornista
fix(#2664): useColorSlider remove redundant PageUp/PageDown keyboard …
d9fbb44 
…events

Also, simplify snapValueToStep.
majornista added a commit that referenced this issue Mar 3, 2022
@majornista
fix(#2664): useColorSliderState remove ternary statement from decrement
06bd74d 
snapValueToStep always starts from minValue.
snowystinger pushed a commit that referenced this issue Mar 4, 2022
@majornista
fix(#2664): useColorSlider: refine PageUp/PageDown behaviors (#2666)
3c89231 
Get colorspace values from the actual colorspace instead of defaults
Remove step from colorfield and colorslider
Fix previous formatted value appearing quickly after blur
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(#2664): useColorSlider: refine PageUp/PageDown behaviors adobe/react-spectrum
1 participant
@majornista