Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions packages/react-core/src/components/Slider/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ export interface SliderProps extends Omit<React.HTMLProps<HTMLDivElement>, 'onCh
className?: string;
/** Array of custom slider step objects (value and label of each step) for the slider. */
customSteps?: SliderStepObject[];
/* Adds a tooltip over the slider thumb containing the current value. */
/** Enables a tooltip over the silder thumb. Defaults to the current value, or tooltipContent if provided. */
hasTooltipOverThumb?: boolean;
/** Content of the tooltip over the slider thumb. Defaults to the current value. */
tooltipContent?: React.ReactNode;
/** Accessible label for the input field. */
inputAriaLabel?: string;
/** Text label that is place after the input field. */
Expand Down Expand Up @@ -81,8 +83,10 @@ export interface SliderProps extends Omit<React.HTMLProps<HTMLDivElement>, 'onCh
showTicks?: boolean;
/** The step interval. */
step?: number;
/* Accessible label for the slider thumb. */
/** Accessible label for the slider thumb. */
thumbAriaLabel?: string;
/** Accessible text for the current value of the slider. Defaults to the current value. */
thumbAriaValueText?: string;
/** Current value of the slider. */
value?: number;
Comment on lines 90 to 91

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.

Let's also add a tooltipProps to spread to the Tooltip

}
Expand All @@ -100,7 +104,9 @@ export const Slider: React.FunctionComponent<SliderProps> = ({
inputLabel,
inputAriaLabel = 'Slider value input',
thumbAriaLabel = 'Value',
thumbAriaValueText,
hasTooltipOverThumb = false,
tooltipContent,
inputPosition = 'end',
onChange,
leftActions,
Expand Down Expand Up @@ -425,8 +431,8 @@ export const Slider: React.FunctionComponent<SliderProps> = ({
aria-valuemin={customSteps ? customSteps[0].value : min}
aria-valuemax={customSteps ? customSteps[customSteps.length - 1].value : max}
aria-valuenow={localValue}
aria-valuetext={findAriaTextValue()}
aria-label={thumbAriaLabel}
aria-valuetext={thumbAriaValueText ? thumbAriaValueText : findAriaTextValue()}

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.

Nit:

Suggested change
aria-valuetext={thumbAriaValueText ? thumbAriaValueText : findAriaTextValue()}
aria-valuetext={thumbAriaValueText ?? findAriaTextValue()}

aria-label={thumbAriaLabel ? thumbAriaLabel : thumbAriaLabel}

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.

Did you mean to use a different value for one of these?

aria-disabled={isDisabled}
aria-describedby={ariaDescribedby}
aria-labelledby={ariaLabelledby}
Expand Down Expand Up @@ -477,7 +483,7 @@ export const Slider: React.FunctionComponent<SliderProps> = ({
className={css('pf-v6-m-tabular-nums')}
triggerRef={thumbRef}
entryDelay={0}
content={findAriaTextValue()}
content={tooltipContent ? tooltipContent : findAriaTextValue()}

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.

Same nit as above

>
{thumbComponent}
</Tooltip>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Slider } from '../Slider';
import { Button } from '../../Button';

Expand Down Expand Up @@ -77,6 +78,17 @@ describe('slider', () => {
const { asFragment } = render(<Slider value={50} hasTooltipOverThumb />);
expect(asFragment()).toMatchSnapshot();
});

test('renders slider with custom tooltip content on thumb', async () => {
const user = userEvent.setup();

render(<Slider value={50} hasTooltipOverThumb tooltipContent="Custom tooltip content" />);

await user.hover(screen.getByRole('slider'));

await screen.findByRole('tooltip');
expect(screen.getByRole('tooltip')).toHaveTextContent('Custom tooltip content');
});
});

test('renders slider with aria-labelledby', () => {
Expand Down Expand Up @@ -104,3 +116,11 @@ test('renders slider with aria-describedby', () => {

expect(slider).toBeVisible();
});

test('renders slider with thumbAriaValueText', () => {
render(<Slider value={50} thumbAriaValueText="Half capacity" />);

const slider = screen.getByRole('slider');

expect(slider).toHaveAttribute('aria-valuetext', 'Half capacity');
});
6 changes: 6 additions & 0 deletions packages/react-core/src/components/Slider/examples/Slider.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ import LockOpenIcon from '@patternfly/react-icons/dist/esm/icons/lock-open-icon'

```

### Custom step tooltip

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.

To add some example verbiage:

You can customize the content of the tooltip by passing the tooltipContent property. By default this tooltip will act as a description to the slider thumb, and thus shouldn't include critical information about the current slider step unless that information is part of the step's aria-valuetext.

If instead you want the tooltip to act as the human-readable value of the slider step - such as when all slider step labels are hidden - you must also pass the thumbAriaValueText property with the same string value as the tooltipContent. Additionally, you should pass the tooltip props object {aria: 'none', 'aria-live': 'off'} to tooltipProps in order to help prevent duplicate announcement from assistive technologies.


```ts file="./SliderCustomTooltip.tsx"

```

## Types

### SliderOnChangeEvent
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { useState } from 'react';
import { Slider, SliderOnChangeEvent } from '@patternfly/react-core';

export const SliderCustomTooltip: React.FunctionComponent = () => {
const [value, setValue] = useState(50);
const steps = [
{ value: 0, label: '0' },
{ value: 12.5, label: '1', isLabelHidden: true },
{ value: 25, label: '2' },
{ value: 37.5, label: '3', isLabelHidden: true },
{ value: 50, label: '4' },
{ value: 62.5, label: '5', isLabelHidden: true },
{ value: 75, label: '6' },
{ value: 87.5, label: '7', isLabelHidden: true },
{ value: 100, label: '8' }
];

const displayValue = () => {
const step = steps.find((step) => step.value === value);
return step ? step.label : 0;
};

const customTooltipContent = () => <div>Custom tooltip content for step: {displayValue()}</div>;

return (
<Slider
hasTooltipOverThumb
tooltipContent={customTooltipContent()}
value={value}
onChange={(_event: SliderOnChangeEvent, value: number) => setValue(value)}
customSteps={steps}
/>
Comment on lines +26 to +32

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.

May not be absolutely necessary with the above example verbiage added in, but we could add a second slider tothis example showing an implementation more similar to the original issue, where each step is a date timestamp. If we do, maybe we have the slider steps min and max values be 1-10 with whole number intervals

);
};
Loading