core(Slider): showBoundaries should be independent from showTicks

[KKoukiou]

Add examples for the valid combinations.

Closes #5739

[patternfly-build]

PF4 preview: https://patternfly-react-pr-5740.surge.sh
A11y report: https://patternfly-react-pr-5740-a11y.surge.sh

[tlabaj]

@mcarrano do we want to show ticks for the min max labels?

[mcarrano]

@tlabaj I don't feel strongly about whether we need ticks at either end of the slider when displaying boundary values. I think that it's OK without them. However, I do think that the last example:

is a bit odd because I generally expect to see tick marks to reflect the intervals on a discrete slider.

@KKoukiou is there a specific reason why you need/want this change for Cockpit?

[KKoukiou]

@mcarrano thanks for the quick answer. Here is a current dialog that uses sliders in cockpit, not PF4 yet.

There are around 10000 points there, showing ticks does not make sense. However I still want to show the boundaries.

[KKoukiou]

Here is another dialog with wide range of input values, where ticks don't belong, however boundaries would make sense.

[mcarrano]

Are these examples discrete or continuous sliders @KKoukiou . If they are continuous, then I agree that tick marks at either end are not needed. If they are discrete sliders, shouldn't we always have the tick marks to indicate slider steps?

I think this example makes sense:

But I question whether this one is needed:

[KKoukiou]

@mcarrano the step in our examples is always 1. So I guess it's continuous, did not realize the difference till now to be honest.
Why do we even need the continuous variant in the end?
To me it's the same like:

        <Slider value={this.state.value3} onChange={this.onChange3} min={0} max={100} step={1}  />

[KKoukiou]

@mcarrano so actualy the first usage scenario I sent (VM memory) has min value != 0, so it's not complying with the continuous variant as far as I understand.

[mcarrano]

I didn't think that the continuous example required the min value to be 0, but perhaps I'm mistaken. I guess the difference between a slider with a step value = 1 and a continuous slider is that the later can return a fractional value, but maybe we don't need both. What are your thoughts @tlabaj ? If someone wants a slider that can only return whole number values, what you you expect them to use?

In any case, I am fine with the essence of this PR. I just want to make sure that the examples are reflecting the way we want people to use this.

[KKoukiou]

@mcarrano @tlabaj the continuous slider as seen in the example code takes values [0, 100], so there are essentially 100 continuous points. In my use case I have arbitary values with min > 0 and max > 100. It can be [128, 512] the min max limits. The step I need is one, but I can't see how i can use the continuous slider here. And I don't want to show 512-128 = 384 ticks.

[mcarrano]

Can't the min and max values for the continuous slider be set to anything @tlabaj ?

[tlabaj]

@mcarrano @KKoukiou in case where you have custom steps, the areCustomStepsContinuous prop will calculate the value based on the position on the slider given the first and last steps. if you are using the min/max/step api, the default step is 1. You can change that so it is less than 1 if you wish. I think @KKoukiou want to show the min max without having to use the customSteps interface which I think is a a valid use case.
This example in core show ticks with a min and max value. In that case you do not want to snap to the value of the step. That is why the areCustomStepsContinuous would be used in that use case. That is also why I asked if we want to show ticks for the min max values.
https://pf4.patternfly.org/components/slider#continuous

[mcarrano]

I'm fine with whatever seems to work best here. Regarding your original question, no I don't think it's necessary to show tick marks for the min and max values @tlabaj .

[mcarrano]

Looks good to me.

[nicolethoen]

I am seeing that the starting values are NaN in the new examples:

[nicolethoen]

@KKoukiou if you address this latest comment, I can merge this :)

[KKoukiou]

@KKoukiou if you address this latest comment, I can merge this :)

Ups sorry , missed your previous ping @nicolethoen :)