mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-11-08 23:58:13 +08:00
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)
This commit is contained in:
@ -639,6 +639,51 @@ export class Range implements ComponentInterface {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private onKnobFocus = (knob: KnobName) => {
|
||||||
|
if (!this.hasFocus) {
|
||||||
|
this.hasFocus = true;
|
||||||
|
this.ionFocus.emit();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Manually manage ion-focused class for dual knobs
|
||||||
|
if (this.dualKnobs && this.el.shadowRoot) {
|
||||||
|
const knobA = this.el.shadowRoot.querySelector('.range-knob-a');
|
||||||
|
const knobB = this.el.shadowRoot.querySelector('.range-knob-b');
|
||||||
|
|
||||||
|
// Remove ion-focused from both knobs first
|
||||||
|
knobA?.classList.remove('ion-focused');
|
||||||
|
knobB?.classList.remove('ion-focused');
|
||||||
|
|
||||||
|
// Add ion-focused only to the focused knob
|
||||||
|
const focusedKnobEl = knob === 'A' ? knobA : knobB;
|
||||||
|
focusedKnobEl?.classList.add('ion-focused');
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
private onKnobBlur = () => {
|
||||||
|
// Check if focus is moving to another knob within the same range
|
||||||
|
// by delaying the reset to allow the new focus to register
|
||||||
|
setTimeout(() => {
|
||||||
|
const activeElement = this.el.shadowRoot?.activeElement;
|
||||||
|
const isStillFocusedOnKnob = activeElement && activeElement.classList.contains('range-knob-handle');
|
||||||
|
|
||||||
|
if (!isStillFocusedOnKnob) {
|
||||||
|
if (this.hasFocus) {
|
||||||
|
this.hasFocus = false;
|
||||||
|
this.ionBlur.emit();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Remove ion-focused from both knobs when focus leaves the range
|
||||||
|
if (this.dualKnobs && this.el.shadowRoot) {
|
||||||
|
const knobA = this.el.shadowRoot.querySelector('.range-knob-a');
|
||||||
|
const knobB = this.el.shadowRoot.querySelector('.range-knob-b');
|
||||||
|
knobA?.classList.remove('ion-focused');
|
||||||
|
knobB?.classList.remove('ion-focused');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}, 0);
|
||||||
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns true if content was passed to the "start" slot
|
* Returns true if content was passed to the "start" slot
|
||||||
*/
|
*/
|
||||||
@ -813,6 +858,8 @@ export class Range implements ComponentInterface {
|
|||||||
min,
|
min,
|
||||||
max,
|
max,
|
||||||
inheritedAttributes,
|
inheritedAttributes,
|
||||||
|
onKnobFocus: this.onKnobFocus,
|
||||||
|
onKnobBlur: this.onKnobBlur,
|
||||||
})}
|
})}
|
||||||
|
|
||||||
{this.dualKnobs &&
|
{this.dualKnobs &&
|
||||||
@ -828,6 +875,8 @@ export class Range implements ComponentInterface {
|
|||||||
min,
|
min,
|
||||||
max,
|
max,
|
||||||
inheritedAttributes,
|
inheritedAttributes,
|
||||||
|
onKnobFocus: this.onKnobFocus,
|
||||||
|
onKnobBlur: this.onKnobBlur,
|
||||||
})}
|
})}
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
@ -908,11 +957,27 @@ interface RangeKnob {
|
|||||||
pinFormatter: PinFormatter;
|
pinFormatter: PinFormatter;
|
||||||
inheritedAttributes: Attributes;
|
inheritedAttributes: Attributes;
|
||||||
handleKeyboard: (name: KnobName, isIncrease: boolean) => void;
|
handleKeyboard: (name: KnobName, isIncrease: boolean) => void;
|
||||||
|
onKnobFocus: (knob: KnobName) => void;
|
||||||
|
onKnobBlur: () => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
const renderKnob = (
|
const renderKnob = (
|
||||||
rtl: boolean,
|
rtl: boolean,
|
||||||
{ knob, value, ratio, min, max, disabled, pressed, pin, handleKeyboard, pinFormatter, inheritedAttributes }: RangeKnob
|
{
|
||||||
|
knob,
|
||||||
|
value,
|
||||||
|
ratio,
|
||||||
|
min,
|
||||||
|
max,
|
||||||
|
disabled,
|
||||||
|
pressed,
|
||||||
|
pin,
|
||||||
|
handleKeyboard,
|
||||||
|
pinFormatter,
|
||||||
|
inheritedAttributes,
|
||||||
|
onKnobFocus,
|
||||||
|
onKnobBlur,
|
||||||
|
}: RangeKnob
|
||||||
) => {
|
) => {
|
||||||
const start = rtl ? 'right' : 'left';
|
const start = rtl ? 'right' : 'left';
|
||||||
|
|
||||||
@ -941,6 +1006,8 @@ const renderKnob = (
|
|||||||
ev.stopPropagation();
|
ev.stopPropagation();
|
||||||
}
|
}
|
||||||
}}
|
}}
|
||||||
|
onFocus={() => onKnobFocus(knob)}
|
||||||
|
onBlur={onKnobBlur}
|
||||||
class={{
|
class={{
|
||||||
'range-knob-handle': true,
|
'range-knob-handle': true,
|
||||||
'range-knob-a': knob === 'A',
|
'range-knob-a': knob === 'A',
|
||||||
|
|||||||
@ -80,6 +80,10 @@
|
|||||||
lower: '10',
|
lower: '10',
|
||||||
upper: '90',
|
upper: '90',
|
||||||
};
|
};
|
||||||
|
|
||||||
|
dualKnobs.addEventListener('ionFocus', () => {
|
||||||
|
console.log('Dual Knob ionFocus', dualKnobs.value);
|
||||||
|
});
|
||||||
</script>
|
</script>
|
||||||
</body>
|
</body>
|
||||||
</html>
|
</html>
|
||||||
|
|||||||
244
core/src/components/range/test/basic/range.spec.ts
Normal file
244
core/src/components/range/test/basic/range.spec.ts
Normal file
@ -0,0 +1,244 @@
|
|||||||
|
import { newSpecPage } from '@stencil/core/testing';
|
||||||
|
|
||||||
|
import { Range } from '../../range';
|
||||||
|
|
||||||
|
describe('range: dual knobs focus management', () => {
|
||||||
|
it('should properly manage initial focus with dual knobs', async () => {
|
||||||
|
const page = await newSpecPage({
|
||||||
|
components: [Range],
|
||||||
|
html: `
|
||||||
|
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
|
||||||
|
</ion-range>
|
||||||
|
`,
|
||||||
|
});
|
||||||
|
|
||||||
|
const range = page.body.querySelector('ion-range');
|
||||||
|
expect(range).not.toBeNull();
|
||||||
|
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// Get the knob elements
|
||||||
|
const knobA = range!.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
|
||||||
|
const knobB = range!.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
|
||||||
|
|
||||||
|
expect(knobA).not.toBeNull();
|
||||||
|
expect(knobB).not.toBeNull();
|
||||||
|
|
||||||
|
// Initially, neither knob should have the ion-focused class
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(false);
|
||||||
|
expect(knobB.classList.contains('ion-focused')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should show focus on the correct knob when focused via keyboard navigation', async () => {
|
||||||
|
const page = await newSpecPage({
|
||||||
|
components: [Range],
|
||||||
|
html: `
|
||||||
|
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
|
||||||
|
</ion-range>
|
||||||
|
`,
|
||||||
|
});
|
||||||
|
|
||||||
|
const range = page.body.querySelector('ion-range');
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
const knobA = range!.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
|
||||||
|
const knobB = range!.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
|
||||||
|
|
||||||
|
// Focus knob A
|
||||||
|
knobA.dispatchEvent(new Event('focus'));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// Only knob A should have the ion-focused class
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(true);
|
||||||
|
expect(knobB.classList.contains('ion-focused')).toBe(false);
|
||||||
|
|
||||||
|
// Focus knob B
|
||||||
|
knobB.dispatchEvent(new Event('focus'));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// Only knob B should have the ion-focused class
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(false);
|
||||||
|
expect(knobB.classList.contains('ion-focused')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should remove focus from all knobs when focus leaves the range', async () => {
|
||||||
|
const page = await newSpecPage({
|
||||||
|
components: [Range],
|
||||||
|
html: `
|
||||||
|
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
|
||||||
|
</ion-range>
|
||||||
|
`,
|
||||||
|
});
|
||||||
|
|
||||||
|
const range = page.body.querySelector('ion-range');
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
const knobA = range!.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
|
||||||
|
const knobB = range!.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
|
||||||
|
|
||||||
|
// Focus knob A
|
||||||
|
knobA.dispatchEvent(new Event('focus'));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(true);
|
||||||
|
|
||||||
|
// Blur the knob (focus leaves the range)
|
||||||
|
knobA.dispatchEvent(new Event('blur'));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// Wait for the timeout in onKnobBlur to complete
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// Neither knob should have the ion-focused class
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(false);
|
||||||
|
expect(knobB.classList.contains('ion-focused')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should emit ionFocus when any knob receives focus but only once until blur', async () => {
|
||||||
|
const page = await newSpecPage({
|
||||||
|
components: [Range],
|
||||||
|
html: `
|
||||||
|
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
|
||||||
|
</ion-range>
|
||||||
|
`,
|
||||||
|
});
|
||||||
|
|
||||||
|
const range = page.body.querySelector('ion-range')!;
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
let focusEventFiredCount = 0;
|
||||||
|
range.addEventListener('ionFocus', () => {
|
||||||
|
focusEventFiredCount++;
|
||||||
|
});
|
||||||
|
|
||||||
|
const knobA = range.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
|
||||||
|
const knobB = range.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
|
||||||
|
|
||||||
|
// Focus knob A
|
||||||
|
knobA.dispatchEvent(new Event('focus'));
|
||||||
|
knobB.dispatchEvent(new Event('focus'));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
expect(focusEventFiredCount).toBe(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should emit ionBlur when focus leaves the range completely', async () => {
|
||||||
|
const page = await newSpecPage({
|
||||||
|
components: [Range],
|
||||||
|
html: `
|
||||||
|
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
|
||||||
|
</ion-range>
|
||||||
|
`,
|
||||||
|
});
|
||||||
|
|
||||||
|
const range = page.body.querySelector('ion-range')!;
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
let blurEventFired = false;
|
||||||
|
range.addEventListener('ionBlur', () => {
|
||||||
|
blurEventFired = true;
|
||||||
|
});
|
||||||
|
|
||||||
|
const knobA = range.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
|
||||||
|
|
||||||
|
// Focus and then blur knob A
|
||||||
|
knobA.dispatchEvent(new Event('focus'));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
knobA.dispatchEvent(new Event('blur'));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// Wait for the timeout in onKnobBlur to complete
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
expect(blurEventFired).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should correctly handle Tab navigation between knobs using KeyboardEvent', async () => {
|
||||||
|
// Using KeyboardEvent to simulate Tab key is more realistic than just firing focus events
|
||||||
|
// because it tests the actual keyboard navigation behavior users would experience
|
||||||
|
const page = await newSpecPage({
|
||||||
|
components: [Range],
|
||||||
|
html: `
|
||||||
|
<button id="before">Before</button>
|
||||||
|
<ion-range dual-knobs="true" min="0" max="100" value='{"lower": 25, "upper": 75}' aria-label="Dual range">
|
||||||
|
</ion-range>
|
||||||
|
<button id="after">After</button>
|
||||||
|
`,
|
||||||
|
});
|
||||||
|
|
||||||
|
const range = page.body.querySelector('ion-range')!;
|
||||||
|
const beforeButton = page.body.querySelector('#before') as HTMLElement;
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
const knobA = range.shadowRoot!.querySelector('.range-knob-a') as HTMLElement;
|
||||||
|
const knobB = range.shadowRoot!.querySelector('.range-knob-b') as HTMLElement;
|
||||||
|
|
||||||
|
// Start with focus on element before the range
|
||||||
|
beforeButton.focus();
|
||||||
|
|
||||||
|
// Simulate Tab key press - this would move focus to first knob
|
||||||
|
let tabEvent = new KeyboardEvent('keydown', {
|
||||||
|
key: 'Tab',
|
||||||
|
code: 'Tab',
|
||||||
|
bubbles: true,
|
||||||
|
cancelable: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeButton.dispatchEvent(tabEvent);
|
||||||
|
knobA.focus(); // Browser would focus next tabindex element
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// First knob should be focused
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(true);
|
||||||
|
expect(knobB.classList.contains('ion-focused')).toBe(false);
|
||||||
|
|
||||||
|
// Simulate another Tab key press - this would move focus to second knob
|
||||||
|
tabEvent = new KeyboardEvent('keydown', {
|
||||||
|
key: 'Tab',
|
||||||
|
code: 'Tab',
|
||||||
|
bubbles: true,
|
||||||
|
cancelable: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
knobA.dispatchEvent(tabEvent);
|
||||||
|
knobB.focus(); // Browser would focus next tabindex element
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// Second knob should be focused, first should not
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(false);
|
||||||
|
expect(knobB.classList.contains('ion-focused')).toBe(true);
|
||||||
|
|
||||||
|
// Simulate Shift+Tab (reverse tab) - should go back to first knob
|
||||||
|
const shiftTabEvent = new KeyboardEvent('keydown', {
|
||||||
|
key: 'Tab',
|
||||||
|
code: 'Tab',
|
||||||
|
shiftKey: true,
|
||||||
|
bubbles: true,
|
||||||
|
cancelable: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
knobB.dispatchEvent(shiftTabEvent);
|
||||||
|
knobA.focus(); // Browser would focus previous tabindex element
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// First knob should be focused again
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(true);
|
||||||
|
expect(knobB.classList.contains('ion-focused')).toBe(false);
|
||||||
|
|
||||||
|
// Verify Arrow key navigation still works on focused knob
|
||||||
|
const arrowEvent = new KeyboardEvent('keydown', {
|
||||||
|
key: 'ArrowRight',
|
||||||
|
code: 'ArrowRight',
|
||||||
|
bubbles: true,
|
||||||
|
cancelable: true,
|
||||||
|
});
|
||||||
|
knobA.dispatchEvent(arrowEvent);
|
||||||
|
await page.waitForChanges();
|
||||||
|
|
||||||
|
// The knob that visually appears focused should be the one that responds to keyboard input
|
||||||
|
expect(knobA.classList.contains('ion-focused')).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user