Skip to content

fix(date-picker): restore focus on date when navigating month with arrow/page keys #9063

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 8 commits into from
Apr 10, 2024
Merged

fix(date-picker): restore focus on date when navigating month with arrow/page keys #9063

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cf783a2
fix(date-picker): restores focus on date when navigation month with a…
anveshmekala Apr 4, 2024
ca378fb
update keys for day wrapper and add tests
anveshmekala Apr 4, 2024
e437f2d
use grid-auto-rows
anveshmekala Apr 5, 2024
13edf90
fix focusout typo
anveshmekala Apr 5, 2024
58fb361
cleanup
anveshmekala Apr 5, 2024
78a685a
add more tests
anveshmekala Apr 5, 2024
8458e01
Merge branch 'main' into anveshmekala/9062-fix-date-picker-arrow-key-…
anveshmekala Apr 8, 2024
c381edc
feedback changes
anveshmekala Apr 10, 2024
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
44 changes: 24 additions & 20 deletions packages/calcite-components/src/components.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ export namespace Components {
*/
"heading": string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -1311,7 +1311,7 @@ export namespace Components {
*/
"activeRange": "start" | "end";
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -1420,6 +1420,10 @@ export namespace Components {
* When `true`, the component is selected.
*/
"selected": boolean;
/**
* Sets focus on the component.
*/
"setFocus": () => Promise<void>;
/**
* Date is the start of date range.
*/
Expand Down Expand Up @@ -1771,7 +1775,7 @@ export namespace Components {
*/
"heading": string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -2144,7 +2148,7 @@ export namespace Components {
*/
"form": string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -3486,7 +3490,7 @@ export namespace Components {
*/
"heading": string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -3556,7 +3560,7 @@ export namespace Components {
*/
"getSelectedItems": () => Promise<Map<string, HTMLCalcitePickListItemElement>>;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -3586,7 +3590,7 @@ export namespace Components {
*/
"groupTitle": string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
}
Expand Down Expand Up @@ -3680,7 +3684,7 @@ export namespace Components {
*/
"heading": string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -5176,7 +5180,7 @@ export namespace Components {
*/
"heading": string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -5204,7 +5208,7 @@ export namespace Components {
*/
"closed": boolean;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel": HeadingLevel;
/**
Expand Down Expand Up @@ -7974,7 +7978,7 @@ declare namespace LocalJSX {
*/
"heading": string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down Expand Up @@ -8743,7 +8747,7 @@ declare namespace LocalJSX {
*/
"activeRange"?: "start" | "end";
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down Expand Up @@ -9225,7 +9229,7 @@ declare namespace LocalJSX {
*/
"heading"?: string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down Expand Up @@ -9617,7 +9621,7 @@ declare namespace LocalJSX {
*/
"form"?: string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down Expand Up @@ -11045,7 +11049,7 @@ declare namespace LocalJSX {
*/
"heading"?: string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down Expand Up @@ -11112,7 +11116,7 @@ declare namespace LocalJSX {
*/
"filteredItems"?: HTMLCalcitePickListItemElement[];
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down Expand Up @@ -11145,7 +11149,7 @@ declare namespace LocalJSX {
*/
"groupTitle"?: string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
}
Expand Down Expand Up @@ -11254,7 +11258,7 @@ declare namespace LocalJSX {
*/
"heading"?: string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down Expand Up @@ -12788,7 +12792,7 @@ declare namespace LocalJSX {
*/
"heading"?: string;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down Expand Up @@ -12820,7 +12824,7 @@ declare namespace LocalJSX {
*/
"closed"?: boolean;
/**
* Specifies the number at which section headings should start.
* Specifies the heading level of the component's `heading` for proper document structure, without affecting visual styling.
*/
"headingLevel"?: HeadingLevel;
/**
Expand Down
29 changes: 27 additions & 2 deletions packages/calcite-components/src/components/date-picker-day/date-picker-day.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Listen,
Prop,
VNode,
Method,
} from "@stencil/core";
import { dateToISO } from "../../utils/date";

Expand All @@ -22,13 +23,19 @@ import {
import { isActivationKey } from "../../utils/key";
import { numberStringFormatter } from "../../utils/locale";
import { Scale } from "../interfaces";
import {
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent,
} from "../../utils/loadable";

@Component({
tag: "calcite-date-picker-day",
styleUrl: "date-picker-day.scss",
shadow: true,
})
export class DatePickerDay implements InteractiveComponent {
export class DatePickerDay implements InteractiveComponent, LoadableComponent {
//--------------------------------------------------------------------------
//
// Properties
Expand Down Expand Up @@ -139,13 +146,31 @@ export class DatePickerDay implements InteractiveComponent {
//
//--------------------------------------------------------------------------

componentWillLoad(): void {
async componentWillLoad(): Promise<void> {
setUpLoadableComponent(this);
this.parentDatePickerEl = closestElementCrossShadowBoundary(
this.el,
"calcite-date-picker",
) as HTMLCalciteDatePickerElement;
}

componentDidLoad(): void {
setComponentLoaded(this);
}

// --------------------------------------------------------------------------
//
// Methods
//
// --------------------------------------------------------------------------

/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentFocusable(this);
this.el.focus();
}

render(): VNode {
const dayId = dateToISO(this.value).replaceAll("-", "");
if (this.parentDatePickerEl) {
Expand Down
10 changes: 5 additions & 5 deletions packages/calcite-components/src/components/date-picker-month/date-picker-month.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
@apply flex
min-w-0
justify-center;
inline-size: calc(100% / 7);

inline-size: 100%;
calcite-date-picker-day {
@apply w-full;
}
Expand All @@ -45,9 +44,10 @@
}

.week-days {
@apply flex
flex-row
py-0;
display: grid;
grid-template-columns: repeat(7, 1fr);
grid-auto-rows: 1fr;
padding-block: 0;
padding-inline: 6px;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be set to a spacing variable?

Copy link
Contributor Author

@anveshmekala anveshmekala Apr 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It is covered in the tokenization PR here

&:focus {
@apply outline-none;
Expand Down
25 changes: 10 additions & 15 deletions packages/calcite-components/src/components/date-picker-month/date-picker-month.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,8 @@ export class DatePickerMonth {
}),
];

const weeks: Day[][] = [];
for (let i = 0; i < days.length; i += 7) {
weeks.push(days.slice(i, i + 7));
}

return (
<Host onFocusOut={this.disableActiveFocus} onKeyDown={this.keyDownHandler}>
<Host onFocusout={this.disableActiveFocus} onKeyDown={this.keyDownHandler}>
<div class="calendar" role="grid">
<div class="week-headers" role="row">
{adjustedWeekDays.map((weekday) => (
Expand All @@ -244,11 +239,10 @@ export class DatePickerMonth {
</span>
))}
</div>
{weeks.map((days) => (
<div class="week-days" role="row">
{days.map((day) => this.renderDateDay(day))}
</div>
))}

<div class="week-days" role="row">
{days.map((day, index) => this.renderDateDay(day, index))}
</div>
</div>
</Host>
);
Expand Down Expand Up @@ -440,15 +434,16 @@ export class DatePickerMonth {
* @param active.day
* @param active.dayInWeek
* @param active.ref
* @param key
*/
private renderDateDay({ active, currentMonth, date, day, dayInWeek, ref }: Day) {
private renderDateDay({ active, currentMonth, date, day, dayInWeek, ref }: Day, key: number) {
const isFocusedOnStart = this.isFocusedOnStart();
const isHoverInRange =
this.isHoverInRange() ||
(!this.endDate && this.hoverRange && sameDate(this.hoverRange?.end, this.startDate));

return (
<div class="day" key={date.toDateString()} role="gridcell">
<div class="day" key={key} role="gridcell">
<calcite-date-picker-day
active={active}
class={{
Expand All @@ -473,10 +468,10 @@ export class DatePickerMonth {
startOfRange={this.isStartOfRange(date)}
value={date}
// eslint-disable-next-line react/jsx-sort-props -- ref should be last so node attrs/props are in sync (see https://github.com/Esri/calcite-design-system/pull/6530)
ref={(el: HTMLCalciteDatePickerDayElement) => {
ref={async (el: HTMLCalciteDatePickerDayElement) => {
// when moving via keyboard, focus must be updated on active date
if (ref && active && this.activeFocus) {
el?.focus();
await el?.setFocus();
Copy link
Member

Choose a reason for hiding this comment

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

do you need to await this? Is ref waiting for the setFocus method to be finished for something? Just trying to understand the need for the ref callback to be async.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, It doesn't need to be async.

}
}}
/>
Expand Down
Loading