Skip to content

Commit 6811fe5

Browse files
ShaneKShaneK
authored
fix(range): improve focus and blur handling for dual knobs (#30482)
Issue number: resolves #internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Currently, if you use keyboard navigation to move between dual range slider knobs, only the first knob you navigate to is highlighted. This is because both elements in the same component are marked as focusable and the code that manages focusable doesn't take into account multiple elements in the same component. https://github.com/user-attachments/assets/36d84eed-6928-446e-becd-ffa2a97e3cc2 ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> After these changes, we manage focusing on dual knob range sliders manually, so using tab navigation through dual knob range sliders focuses knobs as expected. ## Does this introduce a breaking change? - [ ] Yes - [X] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> [Test - Range - Basic screen](https://ionic-framework-git-fw-6401-ionic1.vercel.app/src/components/range/test/basic)
1 parent d52b253 commit 6811fe5

File tree

3 files changed

+316
-1
lines changed

3 files changed

+316
-1
lines changed

‎core/src/components/range/range.tsx‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,51 @@ export class Range implements ComponentInterface {
639639
}
640640
};
641641

642+
private onKnobFocus = (knob: KnobName) => {
643+
if (!this.hasFocus) {
644+
this.hasFocus = true;
645+
this.ionFocus.emit();
646+
}
647+
648+
// Manually manage ion-focused class for dual knobs
649+
if (this.dualKnobs && this.el.shadowRoot) {
650+
const knobA = this.el.shadowRoot.querySelector('.range-knob-a');
651+
const knobB = this.el.shadowRoot.querySelector('.range-knob-b');
652+
653+
// Remove ion-focused from both knobs first
654+
knobA?.classList.remove('ion-focused');
655+
knobB?.classList.remove('ion-focused');
656+
657+
// Add ion-focused only to the focused knob
658+
const focusedKnobEl = knob === 'A' ? knobA : knobB;
659+
focusedKnobEl?.classList.add('ion-focused');
660+
}
661+
};
662+
663+
private onKnobBlur = () => {
664+
// Check if focus is moving to another knob within the same range
665+
// by delaying the reset to allow the new focus to register
666+
setTimeout(() => {
667+
const activeElement = this.el.shadowRoot?.activeElement;
668+
const isStillFocusedOnKnob = activeElement && activeElement.classList.contains('range-knob-handle');
669+
670+
if (!isStillFocusedOnKnob) {
671+
if (this.hasFocus) {
672+
this.hasFocus = false;
673+
this.ionBlur.emit();
674+
}
675+
676+
// Remove ion-focused from both knobs when focus leaves the range
677+
if (this.dualKnobs && this.el.shadowRoot) {
678+
const knobA = this.el.shadowRoot.querySelector('.range-knob-a');
679+
const knobB = this.el.shadowRoot.querySelector('.range-knob-b');
680+
knobA?.classList.remove('ion-focused');
681+
knobB?.classList.remove('ion-focused');
682+
}
683+
}
684+
}, 0);
685+
};
686+
642687
/**
643688
* Returns true if content was passed to the "start" slot
644689
*/
@@ -813,6 +858,8 @@ export class Range implements ComponentInterface {
813858
min,
814859
max,
815860
inheritedAttributes,
861+
onKnobFocus: this.onKnobFocus,
862+
onKnobBlur: this.onKnobBlur,
816863
})}
817864

818865
{this.dualKnobs &&
@@ -828,6 +875,8 @@ export class Range implements ComponentInterface {
828875
min,
829876
max,
830877
inheritedAttributes,
878+
onKnobFocus: this.onKnobFocus,
879+
onKnobBlur: this.onKnobBlur,
831880
})}
832881
</div>
833882
);
@@ -908,11 +957,27 @@ interface RangeKnob {
908957
pinFormatter: PinFormatter;
909958
inheritedAttributes: Attributes;
910959
handleKeyboard: (name: KnobName, isIncrease: boolean) => void;
960+
onKnobFocus: (knob: KnobName) => void;
961+
onKnobBlur: () => void;
911962
}
912963

913964
const renderKnob = (
914965
rtl: boolean,
915-
{ knob, value, ratio, min, max, disabled, pressed, pin, handleKeyboard, pinFormatter, inheritedAttributes }: RangeKnob
966+
{
967+
knob,
968+
value,
969+
ratio,
970+
min,
971+
max,
972+
disabled,
973+
pressed,
974+
pin,
975+
handleKeyboard,
976+
pinFormatter,
977+
inheritedAttributes,
978+
onKnobFocus,
979+
onKnobBlur,
980+
}: RangeKnob
916981
) => {
917982
const start = rtl ? 'right' : 'left';
918983

@@ -941,6 +1006,8 @@ const renderKnob = (
9411006
ev.stopPropagation();
9421007
}
9431008
}}
1009+
onFocus={() => onKnobFocus(knob)}
1010+
onBlur={onKnobBlur}
9441011
class={{
9451012
'range-knob-handle': true,
9461013
'range-knob-a': knob === 'A',

‎core/src/components/range/test/basic/index.html‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ <h2>Pin</h2>
8080
lower: '10',
8181
upper: '90',
8282
};
83+
84+
dualKnobs.addEventListener('ionFocus', () => {
85+
console.log('Dual Knob ionFocus', dualKnobs.value);
86+
});
8387
</script>
8488
</body>
8589
</html>

‎core/src/components/range/test/basic/range.spec.ts‎

Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
import { newSpecPage } from '@stencil/core/testing';
2+
3+
import { Range } from '../../range';
4+
5+
describe('range: dual knobs focus management', () => {
6+
it('should properly manage initial focus with dual knobs', async () => {
7+
const page = await newSpecPage({
8+
components: [Range],
9+
html: `
10+
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
11+
</ion-range>
12+
`,
13+
});
14+
15+
const range = page.body.querySelector('ion-range');
16+
expect(range).not.toBeNull();
17+
18+
await page.waitForChanges();
19+
20+
// Get the knob elements
21+
const knobA = range!.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
22+
const knobB = range!.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
23+
24+
expect(knobA).not.toBeNull();
25+
expect(knobB).not.toBeNull();
26+
27+
// Initially, neither knob should have the ion-focused class
28+
expect(knobA.classList.contains('ion-focused')).toBe(false);
29+
expect(knobB.classList.contains('ion-focused')).toBe(false);
30+
});
31+
32+
it('should show focus on the correct knob when focused via keyboard navigation', async () => {
33+
const page = await newSpecPage({
34+
components: [Range],
35+
html: `
36+
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
37+
</ion-range>
38+
`,
39+
});
40+
41+
const range = page.body.querySelector('ion-range');
42+
await page.waitForChanges();
43+
44+
const knobA = range!.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
45+
const knobB = range!.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
46+
47+
// Focus knob A
48+
knobA.dispatchEvent(new Event('focus'));
49+
await page.waitForChanges();
50+
51+
// Only knob A should have the ion-focused class
52+
expect(knobA.classList.contains('ion-focused')).toBe(true);
53+
expect(knobB.classList.contains('ion-focused')).toBe(false);
54+
55+
// Focus knob B
56+
knobB.dispatchEvent(new Event('focus'));
57+
await page.waitForChanges();
58+
59+
// Only knob B should have the ion-focused class
60+
expect(knobA.classList.contains('ion-focused')).toBe(false);
61+
expect(knobB.classList.contains('ion-focused')).toBe(true);
62+
});
63+
64+
it('should remove focus from all knobs when focus leaves the range', async () => {
65+
const page = await newSpecPage({
66+
components: [Range],
67+
html: `
68+
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
69+
</ion-range>
70+
`,
71+
});
72+
73+
const range = page.body.querySelector('ion-range');
74+
await page.waitForChanges();
75+
76+
const knobA = range!.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
77+
const knobB = range!.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
78+
79+
// Focus knob A
80+
knobA.dispatchEvent(new Event('focus'));
81+
await page.waitForChanges();
82+
83+
expect(knobA.classList.contains('ion-focused')).toBe(true);
84+
85+
// Blur the knob (focus leaves the range)
86+
knobA.dispatchEvent(new Event('blur'));
87+
await page.waitForChanges();
88+
89+
// Wait for the timeout in onKnobBlur to complete
90+
await new Promise((resolve) => setTimeout(resolve, 10));
91+
await page.waitForChanges();
92+
93+
// Neither knob should have the ion-focused class
94+
expect(knobA.classList.contains('ion-focused')).toBe(false);
95+
expect(knobB.classList.contains('ion-focused')).toBe(false);
96+
});
97+
98+
it('should emit ionFocus when any knob receives focus but only once until blur', async () => {
99+
const page = await newSpecPage({
100+
components: [Range],
101+
html: `
102+
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
103+
</ion-range>
104+
`,
105+
});
106+
107+
const range = page.body.querySelector('ion-range')!;
108+
await page.waitForChanges();
109+
110+
let focusEventFiredCount = 0;
111+
range.addEventListener('ionFocus', () => {
112+
focusEventFiredCount++;
113+
});
114+
115+
const knobA = range.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
116+
const knobB = range.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
117+
118+
// Focus knob A
119+
knobA.dispatchEvent(new Event('focus'));
120+
knobB.dispatchEvent(new Event('focus'));
121+
await page.waitForChanges();
122+
123+
expect(focusEventFiredCount).toBe(1);
124+
});
125+
126+
it('should emit ionBlur when focus leaves the range completely', async () => {
127+
const page = await newSpecPage({
128+
components: [Range],
129+
html: `
130+
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
131+
</ion-range>
132+
`,
133+
});
134+
135+
const range = page.body.querySelector('ion-range')!;
136+
await page.waitForChanges();
137+
138+
let blurEventFired = false;
139+
range.addEventListener('ionBlur', () => {
140+
blurEventFired = true;
141+
});
142+
143+
const knobA = range.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
144+
145+
// Focus and then blur knob A
146+
knobA.dispatchEvent(new Event('focus'));
147+
await page.waitForChanges();
148+
149+
knobA.dispatchEvent(new Event('blur'));
150+
await page.waitForChanges();
151+
152+
// Wait for the timeout in onKnobBlur to complete
153+
await new Promise((resolve) => setTimeout(resolve, 10));
154+
await page.waitForChanges();
155+
156+
expect(blurEventFired).toBe(true);
157+
});
158+
159+
it('should correctly handle Tab navigation between knobs using KeyboardEvent', async () => {
160+
// Using KeyboardEvent to simulate Tab key is more realistic than just firing focus events
161+
// because it tests the actual keyboard navigation behavior users would experience
162+
const page = await newSpecPage({
163+
components: [Range],
164+
html: `
165+
<button id="before">Before</button>
166+
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
167+
</ion-range>
168+
<button id="after">After</button>
169+
`,
170+
});
171+
172+
const range = page.body.querySelector('ion-range')!;
173+
const beforeButton = page.body.querySelector('#before') as HTMLElement;
174+
await page.waitForChanges();
175+
176+
const knobA = range.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
177+
const knobB = range.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
178+
179+
// Start with focus on element before the range
180+
beforeButton.focus();
181+
182+
// Simulate Tab key press - this would move focus to first knob
183+
let tabEvent = new KeyboardEvent('keydown', {
184+
key: 'Tab',
185+
code: 'Tab',
186+
bubbles: true,
187+
cancelable: true,
188+
});
189+
190+
beforeButton.dispatchEvent(tabEvent);
191+
knobA.focus(); // Browser would focus next tabindex element
192+
await page.waitForChanges();
193+
194+
// First knob should be focused
195+
expect(knobA.classList.contains('ion-focused')).toBe(true);
196+
expect(knobB.classList.contains('ion-focused')).toBe(false);
197+
198+
// Simulate another Tab key press - this would move focus to second knob
199+
tabEvent = new KeyboardEvent('keydown', {
200+
key: 'Tab',
201+
code: 'Tab',
202+
bubbles: true,
203+
cancelable: true,
204+
});
205+
206+
knobA.dispatchEvent(tabEvent);
207+
knobB.focus(); // Browser would focus next tabindex element
208+
await page.waitForChanges();
209+
210+
// Second knob should be focused, first should not
211+
expect(knobA.classList.contains('ion-focused')).toBe(false);
212+
expect(knobB.classList.contains('ion-focused')).toBe(true);
213+
214+
// Simulate Shift+Tab (reverse tab) - should go back to first knob
215+
const shiftTabEvent = new KeyboardEvent('keydown', {
216+
key: 'Tab',
217+
code: 'Tab',
218+
shiftKey: true,
219+
bubbles: true,
220+
cancelable: true,
221+
});
222+
223+
knobB.dispatchEvent(shiftTabEvent);
224+
knobA.focus(); // Browser would focus previous tabindex element
225+
await page.waitForChanges();
226+
227+
// First knob should be focused again
228+
expect(knobA.classList.contains('ion-focused')).toBe(true);
229+
expect(knobB.classList.contains('ion-focused')).toBe(false);
230+
231+
// Verify Arrow key navigation still works on focused knob
232+
const arrowEvent = new KeyboardEvent('keydown', {
233+
key: 'ArrowRight',
234+
code: 'ArrowRight',
235+
bubbles: true,
236+
cancelable: true,
237+
});
238+
knobA.dispatchEvent(arrowEvent);
239+
await page.waitForChanges();
240+
241+
// The knob that visually appears focused should be the one that responds to keyboard input
242+
expect(knobA.classList.contains('ion-focused')).toBe(true);
243+
});
244+
});

0 commit comments

Comments
 (0)