-
Notifications
You must be signed in to change notification settings - Fork 22
feat(#714): show selected value on range questions #800
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@getodk/web-forms': minor | ||
| --- | ||
|
|
||
| Improved display of range widget to include selected value |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,9 @@ const orientation = props.node.appearances.vertical ? 'vertical' : 'horizontal'; | |
| <div class="range-bound range-min"> | ||
| {{ start }} | ||
| </div> | ||
| <div class="range-value"> | ||
| {{ numberValue }} | ||
| </div> | ||
| <RangeSlider | ||
| :id="node.nodeId" | ||
| :disabled="node.currentState.readonly" | ||
|
|
@@ -88,10 +91,6 @@ const orientation = props.node.appearances.vertical ? 'vertical' : 'horizontal'; | |
| // non-trivial to integrate with PrimeVue's styling. I backed out due to the | ||
| // complexity. | ||
| // | ||
| // - There is no way to indicate the present value. The official MUI (React) | ||
| // component does this with a tooltip which shows when hovering the "thumb" | ||
| // control. | ||
| // | ||
| // - Should we flip either of the following in RTL languages? | ||
| // | ||
| // - Horizontal direction of the control | ||
|
|
@@ -118,41 +117,63 @@ const orientation = props.node.appearances.vertical ? 'vertical' : 'horizontal'; | |
| line-height: 1; | ||
| } | ||
|
|
||
| :deep(.p-slider-handle) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also find the stationary selected value in the center of the range a little easier to use than when it's following the thumb.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of the stationary selected value because it's not clear to me that it's the selected value and not just the value at the centre of the line. Once you start dragging it's obvious, of course, but if a default value is set then it's important that it's obvious what the value is set to.
@lognaturel Do you mean because the thumb obscures the value so it's difficult to set accurately on mobile?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting point! Nicole might have some knowledge of best practices to reference if you want to pull her in. I'm also ok with saying we follow e.g. Material 3 which does pin the value to the thumb: https://m3.material.io/components/sliders/overview
To be explicit, I meant the range thumb as in the circle that you move around to select a value, not a human thumb! I don't think that thumb can ever cover the value. For me it's more about having a single place where I always see the current value if I'm looking for a numeric representation rather than looking at where the thumb is. But then again, maybe the more important thing for range questions is the relative position. I agree with @latin-panda's general observation that styling the value a bit differently from the tick labels would make it clearer that they represent something different. Maybe making it slightly bigger would be enough. I see Material 3 styles it like a callout.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're lightning fast! I'm very happy with that. |
||
| anchor-name: --current-value; | ||
| } | ||
|
|
||
| .range-value { | ||
| position: absolute; | ||
| position-anchor: --current-value; | ||
| } | ||
|
|
||
| &.horizontal { | ||
| height: var(--track-size); | ||
| padding: 0 0.5rem 2lh 0.5rem; | ||
| padding: 2lh 0.5rem 2lh 0.5rem; | ||
|
|
||
| .range-bound { | ||
| bottom: 0; | ||
| } | ||
|
|
||
| .range-min { | ||
| left: 0; | ||
| left: 1px; | ||
| } | ||
|
|
||
| .range-max { | ||
| right: 0; | ||
| right: 4px; | ||
| } | ||
|
|
||
| .range-value { | ||
| justify-self: anchor-center; | ||
| bottom: anchor(top); | ||
| margin-bottom: 0.25rem; | ||
| } | ||
| } | ||
|
|
||
| &.vertical { | ||
| width: var(--track-size); | ||
| padding: 0.5lh 3rem 0.5lh; | ||
| padding: 0.5lh 2lh 0.5lh; | ||
|
|
||
| // Vertical appearance is centered. Consistent with | ||
| // https://docs.getodk.org/form-question-types/#vertical-range-widget | ||
| margin: 0 auto; | ||
|
|
||
| .range-bound { | ||
| right: 0; | ||
| width: 1rem; | ||
| } | ||
|
|
||
| .range-min { | ||
| top: 0; | ||
| top: 1px; | ||
| } | ||
|
|
||
| .range-max { | ||
| bottom: 0; | ||
| bottom: 1px; | ||
| } | ||
|
|
||
| .range-value { | ||
| align-self: anchor-center; | ||
| right: anchor(left); | ||
| margin-right: 0.5rem; | ||
| } | ||
| } | ||
| } | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that the selected value is showing, I've noticed a bit of a glitch:
Sometimes I need to tap the circle several times to select it. This happens in both Chrome and Firefox.
slider-issue.mp4
Is it a quick win we can include in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I bet you're experiencing the issue @gareth pointed out at #777 (comment) Seeing this video makes me think it's more serious than I originally thought.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a way to fix that without changing primevue. The logic to determine the new value is clearly broken which makes it very difficult to select the lowest value. The workaround is to drag the slider which works reliably for me.
I've raised a new issue to track this separately as it's not a quick fix as far as I can see: #801