Skip to content

Commit 77e275c

Browse files
authored
chore: sync with main (#29143)
2 parents d53fb29 + 3c1d8cc commit 77e275c

File tree

7 files changed

+230
-50
lines changed

7 files changed

+230
-50
lines changed

.github/ISSUE_TEMPLATE/bug_report.yml

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,10 @@ body:
2020
id: affected-versions
2121
attributes:
2222
label: Ionic Framework Version
23-
description: Which version(s) of Ionic Framework does this issue impact? For Ionic Framework 1.x issues, please use https://github.com/ionic-team/ionic-v1. For Ionic Framework 2.x and 3.x issues, please use https://github.com/ionic-team/ionic-v3.
23+
description: Which version(s) of Ionic Framework does this issue impact? [Ionic Framework 1.x to 6.x are no longer supported](https://ionicframework.com/docs/reference/support#framework-maintenance-and-support-status). For extended support, considering visiting [Ionic's Enterprise offering](https://ionic.io/enterprise).
2424
options:
25-
- v4.x
26-
- v5.x
27-
- v6.x
2825
- v7.x
26+
- v8.x (Beta)
2927
- Nightly
3028
multiple: true
3129
validations:
@@ -51,20 +49,27 @@ body:
5149
id: steps-to-reproduce
5250
attributes:
5351
label: Steps to Reproduce
54-
description: Please explain the steps required to duplicate this issue.
52+
description: Explain the steps required to reproduce this issue.
5553
placeholder: |
56-
1.
57-
2.
58-
3.
54+
1. Go to '...'
55+
2. Click on '...'
56+
3. Observe: '...'
5957
validations:
6058
required: true
6159

6260
- type: input
6361
id: reproduction-url
6462
attributes:
6563
label: Code Reproduction URL
66-
description: Please reproduce this issue in a blank Ionic Framework starter application and provide a link to the repo. Try out our [Getting Started Wizard](https://ionicframework.com/start#basics) to quickly spin up an Ionic Framework starter app. This is the best way to ensure this issue is triaged quickly. Issues without a code reproduction may be closed if the Ionic Team cannot reproduce the issue you are reporting.
64+
description: |
65+
Reproduce this issue in a blank [Ionic Framework starter application](https://ionicframework.com/start#basics) or a Stackblitz example.
66+
67+
You can use the Stackblitz button available on any of the [component playgrounds](https://ionicframework.com/docs/components) to open an editable example. Remember to save your changes to obtain a link to copy.
68+
69+
Reproductions cases must be minimal and focused around the specific problem you are experiencing. This is the best way to ensure this issue is triaged quickly. Issues without a code reproduction may be closed if the Ionic Team cannot reproduce the issue you are reporting.
6770
placeholder: https://github.com/...
71+
validations:
72+
required: true
6873

6974
- type: textarea
7075
id: ionic-info

core/package-lock.json

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/src/components/checkbox/checkbox.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ export class Checkbox implements ComponentInterface {
187187

188188
return (
189189
<Host
190+
aria-checked={indeterminate ? 'mixed' : `${checked}`}
190191
class={createColorClasses(color, {
191192
[mode]: true,
192193
'in-item': hostContext('ion-item', el),

core/src/components/checkbox/test/checkbox.spec.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,18 @@ describe('ion-checkbox: disabled', () => {
3939
expect(checkbox.checked).toBe(false);
4040
});
4141
});
42+
43+
describe('ion-checkbox: indeterminate', () => {
44+
it('should have a mixed value for aria-checked', async () => {
45+
const page = await newSpecPage({
46+
components: [Checkbox],
47+
html: `
48+
<ion-checkbox indeterminate="true">Checkbox</ion-checkbox>
49+
`,
50+
});
51+
52+
const checkbox = page.body.querySelector('ion-checkbox')!;
53+
54+
expect(checkbox.getAttribute('aria-checked')).toBe('mixed');
55+
});
56+
});

core/src/components/range/range.tsx

Lines changed: 136 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,14 @@ export class Range implements ComponentInterface {
275275
el: rangeSlider,
276276
gestureName: 'range',
277277
gesturePriority: 100,
278-
threshold: 0,
279-
onStart: (ev) => this.onStart(ev),
278+
/**
279+
* Provide a threshold since the drag movement
280+
* might be a user scrolling the view.
281+
* If this is true, then the range
282+
* should not move.
283+
*/
284+
threshold: 10,
285+
onStart: () => this.onStart(),
280286
onMove: (ev) => this.onMove(ev),
281287
onEnd: (ev) => this.onEnd(ev),
282288
});
@@ -378,42 +384,101 @@ export class Range implements ComponentInterface {
378384
this.ionChange.emit({ value: this.value });
379385
}
380386

381-
private onStart(detail: GestureDetail) {
382-
const { contentEl } = this;
383-
if (contentEl) {
384-
this.initialContentScrollY = disableContentScrollY(contentEl);
385-
}
387+
/**
388+
* The value should be updated on touch end or
389+
* when the component is being dragged.
390+
* This follows the native behavior of mobile devices.
391+
*
392+
* For example: When the user lifts their finger from the
393+
* screen after tapping the bar or dragging the bar or knob.
394+
*/
395+
private onStart() {
396+
this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) });
397+
}
386398

387-
const rect = (this.rect = this.rangeSlider!.getBoundingClientRect() as any);
399+
/**
400+
* The value should be updated while dragging the
401+
* bar or knob.
402+
*
403+
* While the user is dragging, the view
404+
* should not scroll. This is to prevent the user from
405+
* feeling disoriented while dragging.
406+
*
407+
* The user can scroll on the view if the knob or
408+
* bar is not being dragged.
409+
*
410+
* @param detail The details of the gesture event.
411+
*/
412+
private onMove(detail: GestureDetail) {
413+
const { contentEl, pressedKnob } = this;
388414
const currentX = detail.currentX;
389415

390-
// figure out which knob they started closer to
391-
let ratio = clamp(0, (currentX - rect.left) / rect.width, 1);
392-
if (isRTL(this.el)) {
393-
ratio = 1 - ratio;
416+
/**
417+
* Since the user is dragging on the bar or knob, the view should not scroll.
418+
*
419+
* This only needs to be done once.
420+
*/
421+
if (contentEl && this.initialContentScrollY === undefined) {
422+
this.initialContentScrollY = disableContentScrollY(contentEl);
394423
}
395424

396-
this.pressedKnob = !this.dualKnobs || Math.abs(this.ratioA - ratio) < Math.abs(this.ratioB - ratio) ? 'A' : 'B';
397-
398-
this.setFocus(this.pressedKnob);
425+
/**
426+
* The `pressedKnob` can be undefined if the user just
427+
* started dragging the knob.
428+
*
429+
* This is necessary to determine which knob the user is dragging,
430+
* especially when it's a dual knob.
431+
* Plus, it determines when to apply certain styles.
432+
*
433+
* This only needs to be done once since the knob won't change
434+
* while the user is dragging.
435+
*/
436+
if (pressedKnob === undefined) {
437+
this.setPressedKnob(currentX);
438+
}
399439

400-
// update the active knob's position
401440
this.update(currentX);
402-
403-
this.ionKnobMoveStart.emit({ value: this.ensureValueInBounds(this.value) });
404441
}
405442

406-
private onMove(detail: GestureDetail) {
407-
this.update(detail.currentX);
408-
}
409-
410-
private onEnd(detail: GestureDetail) {
443+
/**
444+
* The value should be updated on touch end:
445+
* - When the user lifts their finger from the screen after
446+
* tapping the bar.
447+
*
448+
* @param detail The details of the gesture or mouse event.
449+
*/
450+
private onEnd(detail: GestureDetail | MouseEvent) {
411451
const { contentEl, initialContentScrollY } = this;
412-
if (contentEl) {
452+
const currentX = (detail as GestureDetail).currentX || (detail as MouseEvent).clientX;
453+
454+
/**
455+
* The `pressedKnob` can be undefined if the user never
456+
* dragged the knob. They just tapped on the bar.
457+
*
458+
* This is necessary to determine which knob the user is changing,
459+
* especially when it's a dual knob.
460+
* Plus, it determines when to apply certain styles.
461+
*/
462+
if (this.pressedKnob === undefined) {
463+
this.setPressedKnob(currentX);
464+
}
465+
466+
/**
467+
* The user is no longer dragging the bar or
468+
* knob (if they were dragging it).
469+
*
470+
* The user can now scroll on the view in the next gesture event.
471+
*/
472+
if (contentEl && initialContentScrollY !== undefined) {
413473
resetContentScrollY(contentEl, initialContentScrollY);
414474
}
415475

416-
this.update(detail.currentX);
476+
// update the active knob's position
477+
this.update(currentX);
478+
/**
479+
* Reset the pressed knob to undefined since the user
480+
* may start dragging a different knob in the next gesture event.
481+
*/
417482
this.pressedKnob = undefined;
418483

419484
this.emitValueChange();
@@ -445,6 +510,19 @@ export class Range implements ComponentInterface {
445510
this.updateValue();
446511
}
447512

513+
private setPressedKnob(currentX: number) {
514+
const rect = (this.rect = this.rangeSlider!.getBoundingClientRect() as any);
515+
516+
// figure out which knob they started closer to
517+
let ratio = clamp(0, (currentX - rect.left) / rect.width, 1);
518+
if (isRTL(this.el)) {
519+
ratio = 1 - ratio;
520+
}
521+
this.pressedKnob = !this.dualKnobs || Math.abs(this.ratioA - ratio) < Math.abs(this.ratioB - ratio) ? 'A' : 'B';
522+
523+
this.setFocus(this.pressedKnob);
524+
}
525+
448526
private get valA() {
449527
return ratioToValue(this.ratioA, this.min, this.max, this.step);
450528
}
@@ -643,7 +721,39 @@ export class Range implements ComponentInterface {
643721
}
644722

645723
return (
646-
<div class="range-slider" ref={(rangeEl) => (this.rangeSlider = rangeEl)}>
724+
<div
725+
class="range-slider"
726+
ref={(rangeEl) => (this.rangeSlider = rangeEl)}
727+
/**
728+
* Since the gesture has a threshold, the value
729+
* won't change until the user has dragged past
730+
* the threshold. This is to prevent the range
731+
* from moving when the user is scrolling.
732+
*
733+
* This results in the value not being updated
734+
* and the event emitters not being triggered
735+
* if the user taps on the range. This is why
736+
* we need to listen for the "pointerUp" event.
737+
*/
738+
onPointerUp={(ev: PointerEvent) => {
739+
/**
740+
* If the user drags the knob on the web
741+
* version (does not occur on mobile),
742+
* the "pointerUp" event will be triggered
743+
* along with the gesture's events.
744+
* This leads to duplicate events.
745+
*
746+
* By checking if the pressedKnob is undefined,
747+
* we can determine if the "pointerUp" event was
748+
* triggered by a tap or a drag. If it was
749+
* dragged, the pressedKnob will be defined.
750+
*/
751+
if (this.pressedKnob === undefined) {
752+
this.onStart();
753+
this.onEnd(ev);
754+
}
755+
}}
756+
>
647757
{ticks.map((tick) => (
648758
<div
649759
style={tickStyle(tick)}

core/src/components/range/test/range-events.e2e.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,39 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
6767
expect(rangeEnd).toHaveReceivedEventDetail({ value: 21 });
6868
});
6969

70+
test('should emit end event on tap', async ({ page }, testInfo) => {
71+
testInfo.annotations.push({
72+
type: 'issue',
73+
description: 'https://github.com/ionic-team/ionic-framework/issues/28487',
74+
});
75+
76+
await page.setContent(`<ion-range aria-label="Range" value="20"></ion-range>`, config);
77+
78+
const range = page.locator('ion-range');
79+
const rangeEndSpy = await page.spyOnEvent('ionKnobMoveEnd');
80+
const rangeBoundingBox = await range.boundingBox();
81+
/**
82+
* Coordinates for the click event.
83+
* These need to be near the end of the range
84+
* (or anything that isn't the current value).
85+
*
86+
* The number 50 is arbitrary, but it should be
87+
* less than the width of the range.
88+
*/
89+
const x = rangeBoundingBox!.width - 50;
90+
// The y coordinate is the middle of the range.
91+
const y = rangeBoundingBox!.height / 2;
92+
93+
// Click near the end of the range.
94+
await range.click({
95+
position: { x, y },
96+
});
97+
98+
await rangeEndSpy.next();
99+
100+
expect(rangeEndSpy.length).toBe(1);
101+
});
102+
70103
// TODO FW-2873
71104
test.skip('should not scroll when the knob is swiped', async ({ page, skip }) => {
72105
skip.browser('webkit', 'mouse.wheel is not available in WebKit');

0 commit comments

Comments
 (0)