Skip to content

refactor(datetime): use key to preserve calendar body ref node #25838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 29, 2022
Merged

refactor(datetime): use key to preserve calendar body ref node #25838

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
83b22bf
fix issue with sometimes-null refs on datetime component
alicewriteswrongs Aug 26, 2022
c1c82bf
actually don't change that line
alicewriteswrongs Aug 26, 2022
391ab0b
Merge branch 'main' into main
liamdebeasi Aug 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 5 additions & 15 deletions core/src/components/datetime/datetime.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,18 +629,8 @@ export class Datetime implements ComponentInterface {
return presentation === 'date' || presentation === 'date-time' || presentation === 'time-date';
}

/**
* Stencil sometimes sets calendarBodyRef to null on rerender, even though
* the element is present. Query for it manually as a fallback.
*
* TODO(FW-901) Remove when issue is resolved: https://github.com/ionic-team/stencil/issues/3253
*/
private getCalendarBodyEl = () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and remove this little method, but kept the various if-falsy-early-return checks around the component that look like this:

const calendarBodyRef = this.calendarBodyRef
if (!calendarBodyRef) {
  return;
}

so as to minimize churn (like an alternative would be to replace those intermediate calendarBodyRef variables with references to this.calendarBodyRef, but unfortunately I don't think typescript will narrow the type of properties like that after an early return so they'd all have to have ! put at the end which is yucky)

return this.calendarBodyRef || this.el.shadowRoot?.querySelector('.calendar-body');
};

private initializeKeyboardListeners = () => {
const calendarBodyRef = this.getCalendarBodyEl();
const calendarBodyRef = this.calendarBodyRef;
if (!calendarBodyRef) {
return;
}
Expand Down Expand Up @@ -818,7 +808,7 @@ export class Datetime implements ComponentInterface {
};

private initializeCalendarListener = () => {
const calendarBodyRef = this.getCalendarBodyEl();
const calendarBodyRef = this.calendarBodyRef;
if (!calendarBodyRef) {
return;
}
Expand Down Expand Up @@ -1241,7 +1231,7 @@ export class Datetime implements ComponentInterface {
};

private nextMonth = () => {
const calendarBodyRef = this.getCalendarBodyEl();
const calendarBodyRef = this.calendarBodyRef;
if (!calendarBodyRef) {
return;
}
Expand All @@ -1261,7 +1251,7 @@ export class Datetime implements ComponentInterface {
};

private prevMonth = () => {
const calendarBodyRef = this.getCalendarBodyEl();
const calendarBodyRef = this.calendarBodyRef;
if (!calendarBodyRef) {
return;
}
Expand Down Expand Up @@ -1989,7 +1979,7 @@ export class Datetime implements ComponentInterface {
}
private renderCalendar(mode: Mode) {
return (
<div class="datetime-calendar">
<div class="datetime-calendar" key="datetime-calendar">
{this.renderCalendarHeader(mode)}
{this.renderCalendarBody()}
</div>
Expand Down