mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-11-06 22:29:44 +08:00
fix(picker-column): dynamically change options (#27359)
Issue number: resolves #21763 --------- <!-- Please refer to our contributing documentation for any questions on submitting a pull request, or let us know here if you need any help: https://ionicframework.com/docs/building/contributing --> <!-- Some docs updates need to be made in the `ionic-docs` repo, in a separate PR. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation for details. --> <!-- 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. --> Picker options do not render correctly when dynamically changed. It throws an error when the original options' length is no longer the same as the updated options' length. This is due to `refresh()` being called with `this.optsEl` having a stale list of children. The list doesn't get updated until the upcoming render. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> The column will call `refresh()` when it detects the columns has changed. The call needs to be done after `this.optsEl` gets updated with the correct number of children. `componentShouldUpdate()` will check if options has changed -> re-renders with the updated options -> `componentDidUpdate()` calls `refresh()` based on `componentShouldUpdate()` The standalone test page has been updated to include a way to test this fix. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> It might be beneficial to review the [component lifecycle](https://stenciljs.com/docs/component-lifecycle). Co-authored-by: liamdebeasi <liamdebeasi@users.noreply.github.com> --------- Co-authored-by: ionitron <hi@ionicframework.com>
This commit is contained in:
@ -33,6 +33,14 @@ export class PickerColumnCmp implements ComponentInterface {
|
||||
private rafId?: ReturnType<typeof requestAnimationFrame>;
|
||||
private tmrId?: ReturnType<typeof setTimeout>;
|
||||
private noAnimate = true;
|
||||
// `colDidChange` is a flag that gets set when the column is changed
|
||||
// dynamically. When this flag is set, the column will refresh
|
||||
// after the component re-renders to incorporate the new column data.
|
||||
// This is necessary because `this.refresh` queries for the option elements,
|
||||
// so it needs to wait for the latest elements to be available in the DOM.
|
||||
// Ex: column is created with 3 options. User updates the column data
|
||||
// to have 5 options. The column will still think it only has 3 options.
|
||||
private colDidChange = false;
|
||||
|
||||
@Element() el!: HTMLElement;
|
||||
|
||||
@ -46,7 +54,7 @@ export class PickerColumnCmp implements ComponentInterface {
|
||||
@Prop() col!: PickerColumn;
|
||||
@Watch('col')
|
||||
protected colChanged() {
|
||||
this.refresh();
|
||||
this.colDidChange = true;
|
||||
}
|
||||
|
||||
async connectedCallback() {
|
||||
@ -74,21 +82,32 @@ export class PickerColumnCmp implements ComponentInterface {
|
||||
onEnd: (ev) => this.onEnd(ev),
|
||||
});
|
||||
this.gesture.enable();
|
||||
// Options have not been initialized yet
|
||||
// Animation must be disabled through the `noAnimate` flag
|
||||
// Otherwise, the options will render
|
||||
// at the top of the column and transition down
|
||||
this.tmrId = setTimeout(() => {
|
||||
this.noAnimate = false;
|
||||
// After initialization, `refresh()` will be called
|
||||
// At this point, animation will be enabled. The options will
|
||||
// animate as they are being selected.
|
||||
this.refresh(true);
|
||||
}, 250);
|
||||
}
|
||||
|
||||
componentDidLoad() {
|
||||
const colEl = this.optsEl;
|
||||
if (colEl) {
|
||||
// DOM READ
|
||||
// We perfom a DOM read over a rendered item, this needs to happen after the first render
|
||||
this.optHeight = colEl.firstElementChild ? colEl.firstElementChild.clientHeight : 0;
|
||||
}
|
||||
this.onDomChange();
|
||||
}
|
||||
|
||||
this.refresh();
|
||||
componentDidUpdate() {
|
||||
// Options may have changed since last update.
|
||||
if (this.colDidChange) {
|
||||
// Animation must be disabled through the `onDomChange` parameter.
|
||||
// Otherwise, the recently added options will render
|
||||
// at the top of the column and transition down
|
||||
this.onDomChange(true, false);
|
||||
this.colDidChange = false;
|
||||
}
|
||||
}
|
||||
|
||||
disconnectedCallback() {
|
||||
@ -331,7 +350,7 @@ export class PickerColumnCmp implements ComponentInterface {
|
||||
}
|
||||
}
|
||||
|
||||
private refresh(forceRefresh?: boolean) {
|
||||
private refresh(forceRefresh?: boolean, animated?: boolean) {
|
||||
let min = this.col.options.length - 1;
|
||||
let max = 0;
|
||||
const options = this.col.options;
|
||||
@ -356,11 +375,22 @@ export class PickerColumnCmp implements ComponentInterface {
|
||||
const selectedIndex = clamp(min, this.col.selectedIndex ?? 0, max);
|
||||
if (this.col.prevSelected !== selectedIndex || forceRefresh) {
|
||||
const y = selectedIndex * this.optHeight * -1;
|
||||
const duration = animated ? TRANSITION_DURATION : 0;
|
||||
this.velocity = 0;
|
||||
this.update(y, TRANSITION_DURATION, true);
|
||||
this.update(y, duration, true);
|
||||
}
|
||||
}
|
||||
|
||||
private onDomChange(forceRefresh?: boolean, animated?: boolean) {
|
||||
const colEl = this.optsEl;
|
||||
if (colEl) {
|
||||
// DOM READ
|
||||
// We perfom a DOM read over a rendered item, this needs to happen after the first render or after the the column has changed
|
||||
this.optHeight = colEl.firstElementChild ? colEl.firstElementChild.clientHeight : 0;
|
||||
}
|
||||
this.refresh(forceRefresh, animated);
|
||||
}
|
||||
|
||||
render() {
|
||||
const col = this.col;
|
||||
const mode = getIonMode(this);
|
||||
|
||||
@ -0,0 +1,32 @@
|
||||
import { h } from '@stencil/core';
|
||||
import { newSpecPage } from '@stencil/core/testing';
|
||||
|
||||
import { PickerColumnCmp } from '../picker-column';
|
||||
|
||||
describe('picker-column: dynamic options', () => {
|
||||
/**
|
||||
* Issue: https://github.com/ionic-team/ionic-framework/issues/21763
|
||||
*/
|
||||
it('should add an option', async () => {
|
||||
const defaultOptions = [
|
||||
{ text: 'Dog', value: 'dog' },
|
||||
{ text: 'Cat', value: 'cat' },
|
||||
];
|
||||
|
||||
const page = await newSpecPage({
|
||||
components: [PickerColumnCmp],
|
||||
template: () => <ion-picker-column col={{ options: defaultOptions }}></ion-picker-column>,
|
||||
});
|
||||
|
||||
const pickerCol = page.body.querySelector('ion-picker-column');
|
||||
|
||||
pickerCol.col = {
|
||||
options: [...defaultOptions, { text: 'Carrot', value: 'carrot' }],
|
||||
};
|
||||
|
||||
await page.waitForChanges();
|
||||
|
||||
const pickerOpt = pickerCol.querySelector('.picker-opt:nth(2)');
|
||||
expect(pickerOpt.getAttribute('style')).toContain('transform');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user