ref(scraps): remove deprecated formatLabel option from slider#112690
ref(scraps): remove deprecated formatLabel option from slider#112690
Conversation
github-actions
bot
added
the
Scope: Frontend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 34e97f0. Configure here.
| return ( | ||
| <div className={className} ref={ref}> | ||
| {!showCustomInput && showLabel && <SliderLabel>{labelText}</SliderLabel>} | ||
| {!showCustomInput && <SliderLabel>{labelText}</SliderLabel>} |
There was a problem hiding this comment.
Removing showLabel causes unwanted label in volume sliders
Medium Severity
The showLabel prop was removed from RangeSlider, so the SliderLabel now always renders when !showCustomInput. In volumeSliders.tsx, showLabel={false} was previously passed to hide this label. Since showCustomInput isn't passed there either, the SliderLabel will now appear showing a raw numeric value (e.g. 5000000), which is redundant with the separately rendered VolumeAmount that already displays a nicely formatted version (e.g. "5M"). This is a visual regression on the checkout page.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 34e97f0. Configure here.
There was a problem hiding this comment.
i checked and it looks fine. actually looked broken before, see:
There was a problem hiding this comment.
hmm @natemoo-re what do you think, this is how it looks now:
the 0 and 4 are a bit redundant, so is the 2 probably ...
There was a problem hiding this comment.
Definitely redundant—ideally we'd remove the custom "50,000 included" and "1M" to use the built-in label system and use formatOptions to display the "2" as "200K"?
Tricky because the value doesn't match what we're presenting...
There was a problem hiding this comment.
@TkDodo I'm going to take this for a spin locally and see if I can refactor
There was a problem hiding this comment.
Ugh this is a pain to untangle, will have to pick it up on Monday.
There was a problem hiding this comment.
Mmm untangling this is more of a rabbit hole than anticipated. I will pick this up next week!
There was a problem hiding this comment.
oh I didn’t see your latest comments 😅 . What I did in this commit (e1257b3) is to allow formatOptions: 'hidden' for our slider to completely hide the label, as formatting to custom values with Intl.NumberFormatOptions is not really possible.
Waiting till next week if you have a better idea :)
There was a problem hiding this comment.
@natemoo-re are you fine with this solution?
… 'hidden' for the core slider to hide labels
2 participants
No description provided.