Skip to content

[v12] feat(ui-range-input): rework RangeInput #2318

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
Open
ToMESSKa wants to merge 1 commit into
base: v12
Choose a base branch
from
Open

[v12] feat(ui-range-input): rework RangeInput #2318

ToMESSKa wants to merge 1 commit into from
+94 −129

Conversation

@ToMESSKa
Copy link
Contributor

@ToMESSKa ToMESSKa commented Dec 17, 2025 •
edited
Loading

INSTUI-4805

ISSUE:

  • RangeInput needs to be migarted to the new theming system

TEST PLAN:

@ToMESSKa ToMESSKa changed the title Inst UI 4805 range input rework [v12] feat(ui-form-field): rework RangeInput Dec 17, 2025
@ToMESSKa ToMESSKa self-assigned this Dec 17, 2025
@github-actions
Copy link

github-actions bot commented Dec 17, 2025 •
edited
Loading

PR Preview Action v1.6.3

🚀 View preview at
https://instructure.design/pr-preview/pr-2318/

Built to branch gh-pages at 2025-12-23 11:09 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

ToMESSKa
ToMESSKa commented Dec 17, 2025
View reviewed changes
packages/ui-range-input/src/RangeInput/styles.ts
const thumbPosition = {
deprecated: {
marginTop: `calc(-1 * ${componentTheme.handleSize} / 4)`
marginTop: `calc(-1 * ${componentTheme.handleSize} / 4 - 1px)`
Copy link
Contributor Author

@ToMESSKa ToMESSKa Dec 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

because the track got a '1px' wide border, the thumb position changed too so I adjusted the missing pixels here,

@ToMESSKa ToMESSKa changed the title [v12] feat(ui-form-field): rework RangeInput [v12] feat(ui-range-input): rework RangeInput Dec 17, 2025
@ToMESSKa ToMESSKa force-pushed the INSTUI-4805-range-input-rework branch from 5b5c44f to de126e9 Compare December 17, 2025 15:50
adamlobler
adamlobler requested changes Dec 17, 2025
View reviewed changes
Copy link
Collaborator

@adamlobler adamlobler left a comment

Choose a reason for hiding this comment

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

On the original component the padding around the value tooltip was only a horizontal padding, now its a full padding, we should revert this to the original one:

Image

Other than that, it looks okay. There’s just one more small thing, but we should update the tokens as well since we don’t have a proper solution right now... The focus ring isn’t visible in the dark theme, and we don’t currently have a focus color that fits here. I’ll update the tokens and get back to you with a solution:

Image

@ToMESSKa ToMESSKa force-pushed the INSTUI-4805-range-input-rework branch from de126e9 to 21f414b Compare December 18, 2025 14:44
@ToMESSKa
Copy link
Contributor Author

ToMESSKa commented Dec 18, 2025

On the original component the padding around the value tooltip was only a horizontal padding, now its a full padding, we should revert this to the original one:

Image Other than that, it looks okay. There’s just one more small thing, but we should update the tokens as well since we don’t have a proper solution right now... The focus ring isn’t visible in the dark theme, and we don’t currently have a focus color that fits here. I’ll update the tokens and get back to you with a solution: Image

@adamlobler Now the padding token only applies the horizontal padding, I set the vertical padding to 0. Also the focus ring should be okay now, can you check it please?

@ToMESSKa ToMESSKa requested a review from adamlobler December 18, 2025 14:58
adamlobler
adamlobler approved these changes Dec 18, 2025
View reviewed changes
Copy link
Collaborator

@adamlobler adamlobler left a comment

Choose a reason for hiding this comment

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

The paddings and the focusBorder looks okay, thanks for the modification👌 We just need to make some smaller modifications on the token values but its not on your side.

@ToMESSKa ToMESSKa force-pushed the INSTUI-4805-range-input-rework branch from 21f414b to 5ef95fd Compare December 23, 2025 11:05
ToMESSKa
ToMESSKa commented Dec 23, 2025
View reviewed changes
packages/ui-themes/src/utils/boxShadowObjectToString.ts
Comment on lines +30 to +31
function boxShadowToCSSString(boxShadowObject: TokenBoxshadowValueInst) {
// weird string concatenation is to make it look nice in the debugger
Copy link
Contributor Author

@ToMESSKa ToMESSKa Dec 23, 2025

Choose a reason for hiding this comment

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

this from this PR: #2282

@ToMESSKa ToMESSKa requested review from balzss and removed request for joyenjoyer December 23, 2025 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamlobler adamlobler adamlobler approved these changes

@git-nandor git-nandor Awaiting requested review from git-nandor

@balzss balzss Awaiting requested review from balzss

Assignees

@ToMESSKa ToMESSKa

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ToMESSKa @adamlobler