Skip to content

Add OK basic slider component - #3048

Open
atmgrifter00 with Copilot wants to merge 14 commits into
mainfrom
copilot/add-ok-component-for-fast-slider
Open

Add OK basic slider component#3048
atmgrifter00 with Copilot wants to merge 14 commits into
mainfrom
copilot/add-ok-component-for-fast-slider

Conversation

Copilot AI commented Aug 28, 2026

Copy link
Copy Markdown

Pull Request

🤨 Rationale

Add a Nimble-styled OK slider backed by the FAST Foundation slider.

👩‍💻 Implementation

  • Added and registered ok-basic-slider under ok-components/src/basic.
  • Styled horizontal and vertical layouts using Nimble design tokens.
  • Covered hover, focus, active, readonly, and disabled states.
  • Added Storybook documentation and visual-state matrices.

🧪 Testing

  • Added construction and FAST template integration tests.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>
@atmgrifter00

Copy link
Copy Markdown
Contributor

@copilot can we include the ok-react wrapper for this component as part of this PR?

Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>

Copilot AI commented Aug 28, 2026

Copy link
Copy Markdown
Author

@copilot can we include the ok-react wrapper for this component as part of this PR?

Added the OkBasicSlider wrapper with typed onChange support and package change metadata in b0cb180.

Comment thread change/@ni-ok-react-7f04b43d-439e-4848-b394-511e0909be4d.json
@rajsite

rajsite commented Aug 28, 2026

Copy link
Copy Markdown
Member

Approved the workflow runs

@atmgrifter00

Copy link
Copy Markdown
Contributor

@copilot When pressing the slider it looks like this:
image

I think we should adopt the same styling patterns seen in the switch control (see it's styling CSS file for reference: https://github.com/ni/nimble/blob/main/packages/nimble-components/src/switch/styles.ts.

Normal:
image

Hover:
image

Click:
image

The color of the track for the slider should also match that of the switch.

Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>
Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Author

@copilot When pressing the slider it looks like this:
image

...

Updated the thumb to use the switch indicator’s normal, hover, focus, active-inner, and disabled patterns, and matched the track to the switch’s theme-specific colors in 3577776.

@atmgrifter00

Copy link
Copy Markdown
Contributor

@copilot we also need to provide visual feedback for what the actual value is as you are changing it (perhaps tooltip next to the . We can provide this configuration via an attribute API. Additionally it would be nice to provide a fill effect on the slider from the minimum up to the current value. Let's start with using the $ni-nimble-fill-selected-color token for the fill color.

Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Author

@copilot we also need to provide visual feedback for what the actual value is as you are changing it (perhaps tooltip next to the . We can provide this configuration via an attribute API. Additionally...

Added value-visible for a live tooltip positioned beside and moving with the thumb, and changed the minimum-to-current fill to fillSelectedColor in ae9ad82.

@atmgrifter00

Copy link
Copy Markdown
Contributor

@copilot A few things to fix:
image

The thumb for the slider should not be showing the track behind it when in any mode (disabled or otherwise). The switch may have been able to get away with this because its track is the same size and color as the background of the thumb, but for the slider we can't do that, unless we can provide a mask image that removes the visuals behind the thumb. Let's also make the thumb size about 60% of its current size. We also should remove the visual box around the value. I would also like to see the min and max values to show up on the slider on hover.

Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Author

@copilot A few things to fix:
image

...

Made the thumb opaque in every state, reduced it from 24px to 14px, removed the value box styling, and added hover-only min/max labels at the track ends in 8d2ec57.

@atmgrifter00

Copy link
Copy Markdown
Contributor

@copilot when you hover over the thumb the value text above the thumb shifts slightly down. We should ensure that it does not change position on hover.

Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Author

@copilot when you hover over the thumb the value text above the thumb shifts slightly down. We should ensure that it does not change position on hover.

Kept the thumb border geometry constant and moved the extra hover/focus emphasis to an inset shadow, so the value text no longer shifts. Addressed in 26c2893.

${display('inline-block')}

:host {
width: 240px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@copilot why are you setting an explicit width here? Most of the nimble-components do not do this, and allow themselves to fit the width of the parent they are placed within. Providing a min-width may make sense (I think I would do 3 times the width of the thumb, such that the thumb could have 3 discrete values to snap to where the thumb visual wouldn't overlap itself at any of the steps.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@copilot This applies to the height when oriented vertically.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The 240px width came from the initial standalone presentation and was not a component requirement. I replaced it with parent-responsive width: 100% and a 42px minimum (three 14px thumb widths), while keeping vertical cross-axis sizing independent. Addressed in 10dd851.

Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>
@atmgrifter00

Copy link
Copy Markdown
Contributor

@copilot Let's make one other adjustment: make the fill color for the track from the minimum to the thumb the same as the highlight of the thumb when pressed.

Copilot AI and others added 2 commits August 29, 2026 20:40
Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>
Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>
Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Author

@copilot Let's make one other adjustment: make the fill color for the track from the minimum to the thumb the same as the highlight of the thumb when pressed.

Updated the selected track segment to share the pressed thumb’s theme-specific highlight color, while preserving the disabled override. Addressed in 0d040d0.

@atmgrifter00
atmgrifter00 marked this pull request as ready for review August 29, 2026 20:57
@atmgrifter00
atmgrifter00 requested a review from jattasNI as a code owner August 29, 2026 20:57
Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>
@atmgrifter00

Copy link
Copy Markdown
Contributor

@copilot let's make a slight adjustment and show the min and max values either during hover or while the slider is being operated (clicked).

Co-authored-by: atmgrifter00 <26874831+atmgrifter00@users.noreply.github.com>

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Author

@copilot let's make a slight adjustment and show the min and max values either during hover or while the slider is being operated (clicked).

Updated the range labels to remain visible while the slider is active/clicked. Addressed in 179c2e2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants