Fix/slider input validation

[jamesducky]

Closes [BUG] Slider - Number input validation too restrictive
Matches updated UX spec

Changes

• Number Input field now validates on blur, not on change
• Refactors valid input to be called once, and set in state.
• Refactors validation logic to remove variable mutation
• Calls the permitted input range in state once

Issues

• Tests for update validation removed as validation does not happen until blur on input. Stencil e2e does not provide a way to trigger the blur event. Unable to find work around.

[github-actions]

Visit the preview URL for this PR (updated for commit 1d21a85):

https://test-auth-d13f2--pr65-fix-slider-2-fvnsnvtu.web.app

_{(expires Tue, 13 Sep 2022 08:37:29 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

[nathan-bird]

Tests for update validation removed as validation does not happen until blur on input. Stencil e2e does not provide a way to trigger the blur event. Unable to find work around.

Since Stencil 2.17.2 they have added a mock blur to their event api.
I suggest asking other Plumage devs on how to update the package and how best to try and use this api to write the tests you want.

[nathan-bird]

The input field is not populated with a value on render, it is only updated on slider change.

Keyboard navigation is difficult/impossible.

• Hitting enter when focused on input field does not set the value
• Entering a number starts with an invalid value forces the user out of focus. To repeat; go to Stepped story, tab into the input field, try to type "15". After entering "1" focus will be removed and the slider will move to 5.

[jamesducky]

• Entering a number starts with an invalid value forces the user out of focus. To repeat; go to Stepped story, tab into the input field, try to type "15". After entering "1" focus will be removed and the slider will move to 5.

I think you'll only see this in storybook, not in the React example. The reason is the default keyboard shortcuts in storybook shift focus.

[jamesducky]

Updates

• merged react app example to include tab
• fixed a bug) - zero is a falsey value causing new value to be rejected
• as switching tabs causes the values to reset, lifted state up to parent component (tab). (Pasing the value update function as props to the child )

[nathan-bird]

A few comments for you to check out

[nathan-bird]

On first render I am seeing thumb label positioning problems.

I have seen this happen on all the sliders, it does not appear to be unique to the decimal slider.

[jamesducky]

// Using min-width

Managed to recreate the bug by putting a fixed width on the slider (240px).
The input field is actually set by the 'size' value, which is specified by the max value of the input number field. Forcing a min-width on the input field changes the input field, causing the input field to change size and making it overflow its container.
Using an outline is also causing an additional overflow.
Fix: removed min-width, added a negative outline-offset on the input field
// Without min-width

The reason I was using min-width was to make the input field match the design.

[jamesducky]

The 'track value' (the input range slider container's) width was getting changed between rendering the component container and calculating the thumb position because of flex gap and fit-content. Changing flex gap for margin-left with a fixed value and removing fit-content prevents input from rendering.

[jamesducky]

@nathan-bird: I've added a prop to allow the user to control textInputWidth. This is not because of the thumb positioning issue, which I've fixed separately, but I think we need this to control to maintain visual consistency. As discussed in the spec.

[nathan-bird]

Updates look ok. Would be nice with a test of the new control.

[nathan-bird]

LGTM 🎉

Next time something like this could be many smaller PRs which would make it faster to review and merge.