Skip to content

Commit b8530e5

Browse files
kibanamachinecqliu1Heenawter
authored
[9.1] [Dashboard] Fix keyboard interaction on range slider control (#230893) (#231691)
# Backport This will backport the following commits from `main` to `9.1`: - [[Dashboard] Fix keyboard interaction on range slider control (#230893)](#230893) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Catherine Liu","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-08-13T19:01:11Z","message":"[Dashboard] Fix keyboard interaction on range slider control (#230893)\n\n## Summary\n\nCloses https://github.com/elastic/kibana/issues/183185.\n\nThis enables up/down arrow key inputs on the lower bound range slider\nnumber field. Before we were ignoring the lower and upper bound values\nif they matched the min/max placeholder, respectively. The resulted in\nthe lower bound number field not committing the minimum available value\nand preventing the up arrow keyboard interaction from incrementing the\nvalue. This change treats the selected value as a selection even when it\nmatches the corresponding placeholder.\n\nI noticed when changing either the min or max number field, the\n`onChange` handler would commit both values as selection, so I added an\nadditional check for if the value change originated from the number\nfields and only commit the value for the field that was changed.\n\n\n\nhttps://github.com/user-attachments/assets/f7dfa7a2-fdc7-4f8f-b2cc-5c63bf7fdc1a\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>\nCo-authored-by: Hannah Mudge <[email protected]>","sha":"340e0bc69413bf599edc570323a5a546b3e17dc1","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Dashboard","release_note:fix","Team:Presentation","loe:medium","impact:medium","Project:Controls","backport:version","v9.2.0","v9.1.2"],"title":"[Dashboard] Fix keyboard interaction on range slider control","number":230893,"url":"https://github.com/elastic/kibana/pull/230893","mergeCommit":{"message":"[Dashboard] Fix keyboard interaction on range slider control (#230893)\n\n## Summary\n\nCloses https://github.com/elastic/kibana/issues/183185.\n\nThis enables up/down arrow key inputs on the lower bound range slider\nnumber field. Before we were ignoring the lower and upper bound values\nif they matched the min/max placeholder, respectively. The resulted in\nthe lower bound number field not committing the minimum available value\nand preventing the up arrow keyboard interaction from incrementing the\nvalue. This change treats the selected value as a selection even when it\nmatches the corresponding placeholder.\n\nI noticed when changing either the min or max number field, the\n`onChange` handler would commit both values as selection, so I added an\nadditional check for if the value change originated from the number\nfields and only commit the value for the field that was changed.\n\n\n\nhttps://github.com/user-attachments/assets/f7dfa7a2-fdc7-4f8f-b2cc-5c63bf7fdc1a\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>\nCo-authored-by: Hannah Mudge <[email protected]>","sha":"340e0bc69413bf599edc570323a5a546b3e17dc1"}},"sourceBranch":"main","suggestedTargetBranches":["9.1"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/230893","number":230893,"mergeCommit":{"message":"[Dashboard] Fix keyboard interaction on range slider control (#230893)\n\n## Summary\n\nCloses https://github.com/elastic/kibana/issues/183185.\n\nThis enables up/down arrow key inputs on the lower bound range slider\nnumber field. Before we were ignoring the lower and upper bound values\nif they matched the min/max placeholder, respectively. The resulted in\nthe lower bound number field not committing the minimum available value\nand preventing the up arrow keyboard interaction from incrementing the\nvalue. This change treats the selected value as a selection even when it\nmatches the corresponding placeholder.\n\nI noticed when changing either the min or max number field, the\n`onChange` handler would commit both values as selection, so I added an\nadditional check for if the value change originated from the number\nfields and only commit the value for the field that was changed.\n\n\n\nhttps://github.com/user-attachments/assets/f7dfa7a2-fdc7-4f8f-b2cc-5c63bf7fdc1a\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...\n\n---------\n\nCo-authored-by: kibanamachine <[email protected]>\nCo-authored-by: Hannah Mudge <[email protected]>","sha":"340e0bc69413bf599edc570323a5a546b3e17dc1"}},{"branch":"9.1","label":"v9.1.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Catherine Liu <[email protected]> Co-authored-by: Hannah Mudge <[email protected]>
1 parent 3bb0ff1 commit b8530e5

File tree

4 files changed

+167
-14
lines changed

4 files changed

+167
-14
lines changed
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
import React from 'react';
11+
12+
import { fireEvent, render } from '@testing-library/react';
13+
14+
import { RangeSliderControl, Props as RangeSliderControlProps } from './range_slider_control';
15+
import { RangeValue } from '../types';
16+
17+
describe('RangeSliderControl', () => {
18+
const defaultProps: RangeSliderControlProps = {
19+
compressed: true,
20+
isInvalid: false,
21+
isLoading: false,
22+
fieldName: 'Test field',
23+
max: 100,
24+
min: 0,
25+
step: 1,
26+
uuid: 'test-uuid',
27+
value: undefined,
28+
fieldFormatter: undefined,
29+
onChange: jest.fn(),
30+
};
31+
32+
it('renders range slider without selection', () => {
33+
const rangeSliderControl = render(<RangeSliderControl {...defaultProps} />);
34+
35+
expect(rangeSliderControl.getByTestId('range-slider-control-test-uuid')).toBeInTheDocument();
36+
expect(rangeSliderControl.getByTestId('rangeSlider__lowerBoundFieldNumber')).toHaveAttribute(
37+
'placeholder',
38+
'0'
39+
);
40+
expect(rangeSliderControl.getByTestId('rangeSlider__lowerBoundFieldNumber')).toHaveValue(null);
41+
expect(rangeSliderControl.getByTestId('rangeSlider__upperBoundFieldNumber')).toHaveAttribute(
42+
'placeholder',
43+
'100'
44+
);
45+
expect(rangeSliderControl.getByTestId('rangeSlider__upperBoundFieldNumber')).toHaveValue(null);
46+
expect(
47+
rangeSliderControl.queryByTestId('range-slider-control-invalid-append-test-uuid')
48+
).toBeFalsy();
49+
expect(rangeSliderControl.baseElement.querySelector('.euiLoadingSpinner')).toBeFalsy();
50+
});
51+
52+
it('renders range slider with only lower bound selection', () => {
53+
const props = {
54+
...defaultProps,
55+
value: ['10', ''] as RangeValue,
56+
};
57+
58+
const rangeSliderControl = render(<RangeSliderControl {...props} />);
59+
60+
expect(rangeSliderControl.queryByTestId('range-slider-control-test-uuid')).toBeInTheDocument();
61+
expect(rangeSliderControl.getByTestId('rangeSlider__lowerBoundFieldNumber')).toHaveValue(10);
62+
expect(rangeSliderControl.getByTestId('rangeSlider__upperBoundFieldNumber')).toHaveValue(null);
63+
});
64+
65+
it('renders range slider with only upper bound selection', () => {
66+
const props = {
67+
...defaultProps,
68+
value: ['', '80'] as RangeValue,
69+
};
70+
const rangeSliderControl = render(<RangeSliderControl {...props} />);
71+
72+
expect(rangeSliderControl.queryByTestId('range-slider-control-test-uuid')).toBeInTheDocument();
73+
expect(rangeSliderControl.getByTestId('rangeSlider__lowerBoundFieldNumber')).toHaveValue(null);
74+
expect(rangeSliderControl.getByTestId('rangeSlider__upperBoundFieldNumber')).toHaveValue(80);
75+
});
76+
77+
it('renders range slider with both bounds selected', () => {
78+
const props = {
79+
...defaultProps,
80+
value: ['30', '70'] as RangeValue,
81+
};
82+
const rangeSliderControl = render(<RangeSliderControl {...props} />);
83+
84+
expect(rangeSliderControl.getByTestId('rangeSlider__lowerBoundFieldNumber')).toHaveValue(30);
85+
expect(rangeSliderControl.getByTestId('rangeSlider__upperBoundFieldNumber')).toHaveValue(70);
86+
});
87+
88+
it('renders an invalid indicator when isInvalid is true', () => {
89+
const props = {
90+
...defaultProps,
91+
isInvalid: true,
92+
value: ['110', ''] as RangeValue,
93+
};
94+
const rangeSliderControl = render(<RangeSliderControl {...props} />);
95+
96+
expect(
97+
rangeSliderControl.getByTestId('range-slider-control-invalid-append-test-uuid')
98+
).toBeInTheDocument();
99+
});
100+
101+
it('renders a loading indicator when isLoading is true', () => {
102+
const props = {
103+
...defaultProps,
104+
isLoading: true,
105+
value: ['50', '60'] as RangeValue,
106+
};
107+
const rangeSliderControl = render(<RangeSliderControl {...props} />);
108+
109+
expect(rangeSliderControl.baseElement.querySelector('.euiLoadingSpinner')).toBeInTheDocument();
110+
});
111+
112+
it('updates selection when lower bound input changes', async () => {
113+
const rangeSliderControl = render(<RangeSliderControl {...defaultProps} />);
114+
115+
const lowerBoundInput = rangeSliderControl.getByTestId(
116+
'rangeSlider__lowerBoundFieldNumber'
117+
) as HTMLInputElement;
118+
119+
fireEvent.input(lowerBoundInput, {
120+
target: { value: '40' },
121+
});
122+
123+
expect(lowerBoundInput).toHaveValue(40);
124+
expect(rangeSliderControl.getByTestId('rangeSlider__upperBoundFieldNumber')).toHaveValue(null);
125+
});
126+
127+
it('updates selection when upper bound input changes', async () => {
128+
const rangeSliderControl = render(<RangeSliderControl {...defaultProps} />);
129+
130+
const upperBoundInput = rangeSliderControl.getByTestId(
131+
'rangeSlider__upperBoundFieldNumber'
132+
) as HTMLInputElement;
133+
134+
fireEvent.input(upperBoundInput, {
135+
target: { value: '80' },
136+
});
137+
138+
expect(rangeSliderControl.getByTestId('rangeSlider__lowerBoundFieldNumber')).toHaveValue(null);
139+
expect(rangeSliderControl.getByTestId('rangeSlider__upperBoundFieldNumber')).toHaveValue(80);
140+
});
141+
});

src/platform/plugins/shared/controls/public/controls/data_controls/range_slider/components/range_slider_control.tsx

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { MIN_POPOVER_WIDTH } from '../../../constants';
2222
import { RangeSliderStrings } from '../range_slider_strings';
2323
import { rangeSliderControlStyles } from './range_slider.styles';
2424

25-
interface Props {
25+
export interface Props {
2626
compressed: boolean;
2727
controlPanelClassName?: string;
2828
isInvalid: boolean;
@@ -151,7 +151,7 @@ export const RangeSliderControl: FC<Props> = ({
151151
placeholder: string;
152152
ariaLabel: string;
153153
id: string;
154-
}) => {
154+
}): Partial<EuiDualRangeProps['minInputProps']> => {
155155
return {
156156
isInvalid: undefined, // disabling this prop to handle our own validation styling
157157
placeholder,
@@ -161,7 +161,7 @@ export const RangeSliderControl: FC<Props> = ({
161161
isInvalid ? styles.fieldNumbers.invalid : styles.fieldNumbers.valid,
162162
],
163163
className: 'rangeSliderAnchor__fieldNumber',
164-
value: inputValue === placeholder ? '' : inputValue,
164+
value: inputValue,
165165
title: !isInvalid && step ? '' : undefined, // overwrites native number input validation error when the value falls between two steps
166166
'data-test-subj': `rangeSlider__${testSubj}`,
167167
'aria-label': ariaLabel,
@@ -178,7 +178,7 @@ export const RangeSliderControl: FC<Props> = ({
178178
testSubj: 'lowerBoundFieldNumber',
179179
placeholder: String(min ?? -Infinity),
180180
ariaLabel: RangeSliderStrings.control.getLowerBoundAriaLabel(fieldName),
181-
id: uuid,
181+
id: `${uuid}-lowerBound`,
182182
});
183183
}, [getCommonInputProps, displayedValue, min, fieldName, uuid]);
184184

@@ -188,7 +188,7 @@ export const RangeSliderControl: FC<Props> = ({
188188
testSubj: 'upperBoundFieldNumber',
189189
placeholder: String(max ?? Infinity),
190190
ariaLabel: RangeSliderStrings.control.getUpperBoundAriaLabel(fieldName),
191-
id: uuid,
191+
id: `${uuid}-upperBound`,
192192
});
193193
}, [getCommonInputProps, displayedValue, max, fieldName, uuid]);
194194

@@ -251,7 +251,16 @@ export const RangeSliderControl: FC<Props> = ({
251251
minInputProps={minInputProps}
252252
maxInputProps={maxInputProps}
253253
value={[displayedValue[0] || displayedMin, displayedValue[1] || displayedMax]}
254-
onChange={([minSelection, maxSelection]: [number | string, number | string]) => {
254+
onChange={([minSelection, maxSelection]: [number | string, number | string], _, ev) => {
255+
const originatingInputId = ev?.currentTarget.getAttribute('id');
256+
257+
if (originatingInputId?.includes('lowerBound')) {
258+
// preserve original upper bound selection if only lower bound number field changed
259+
maxSelection = displayedValue[1];
260+
} else if (originatingInputId?.includes('upperBound')) {
261+
// preserve original lower bound selection if only upper bound number field changed
262+
minSelection = displayedValue[0];
263+
}
255264
setDisplayedValue([String(minSelection), String(maxSelection)]);
256265
debouncedOnChange([String(minSelection), String(maxSelection)]);
257266
}}

src/platform/plugins/shared/controls/public/controls/data_controls/range_slider/get_range_slider_control_factory.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,10 +202,10 @@ export const getRangesliderControlFactory = (): DataControlFactory<
202202
const lte = parseFloat(value?.[1] ?? '');
203203

204204
let rangeFilter: Filter | undefined;
205-
if (value && dataView && dataViewField && !isNaN(gte) && !isNaN(lte)) {
205+
if (value && dataView && dataViewField && (!isNaN(gte) || !isNaN(lte))) {
206206
const params = {
207-
gte,
208-
lte,
207+
gte: !isNaN(gte) ? gte : -Infinity,
208+
lte: !isNaN(lte) ? lte : Infinity,
209209
} as RangeFilterParams;
210210

211211
rangeFilter = buildRangeFilter(dataViewField, params, dataView);

src/platform/test/functional/apps/dashboard_elements/controls/common/range_slider.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,17 +196,20 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
196196
it('can select a range on a defined step interval using arrow keys', async () => {
197197
const secondId = (await dashboardControls.getAllControlIds())[1];
198198

199+
// clear out the lower bound selection
200+
await dashboardControls.clearControlSelections(secondId);
201+
199202
await testSubjects.click(
200203
`range-slider-control-${secondId} > rangeSlider__lowerBoundFieldNumber`
201204
);
202205

203206
// use arrow key to set lower bound to the next step up
204-
await browser.pressKeys(browser.keys.ARROW_UP);
205-
await dashboardControls.validateRange('value', secondId, '300', '');
207+
await browser.pressKeys(browser.keys.ARROW_UP, browser.keys.ARROW_UP);
208+
await dashboardControls.validateRange('value', secondId, '200', '');
206209

207210
// use arrow key to set lower bound to the next step up
208211
await browser.pressKeys(browser.keys.ARROW_DOWN);
209-
await dashboardControls.validateRange('value', secondId, '200', '');
212+
await dashboardControls.validateRange('value', secondId, '100', '');
210213

211214
await dashboardControls.rangeSliderSetUpperBound(secondId, '800');
212215

@@ -216,11 +219,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
216219

217220
// use arrow key to set upper bound to the next step up
218221
await browser.pressKeys(browser.keys.ARROW_UP);
219-
await dashboardControls.validateRange('value', secondId, '200', '900');
222+
await dashboardControls.validateRange('value', secondId, '100', '900');
220223

221224
// use arrow key to set upper bound to the next step up
222225
await browser.pressKeys(browser.keys.ARROW_DOWN);
223-
await dashboardControls.validateRange('value', secondId, '200', '800');
226+
await dashboardControls.validateRange('value', secondId, '100', '800');
224227

225228
await dashboard.clearUnsavedChanges();
226229
});

0 commit comments

Comments
 (0)