Closed Bug 1803608 Opened 3 years ago Closed 3 years ago

Datepicker key nav - Clean up the _updateViewIfUndefined and _updateViewIfOutside and _updateKeyboardFocus functions

Categories

(Core :: Layout: Form Controls, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
111 Branch
Tracking Status
firefox110 --- fixed
firefox111 --- fixed

People

(Reporter: ayeddi, Assigned: ayeddi)

References

Details

(Keywords: access)

Attachments

(1 file)

Functions to combine:

  1. _updateViewIfUndefined
  2. _updateViewIfOutside
  3. _updateKeyboardFocus

See the discussion in the Datepicker Part 4 patch when handling keyboard navigation within a calendar grid (in calendar.js), esp. feedback from Jamie and Mike

From Jamie:

There's still something I'm missing here. Nevertheless, I took a stab at combining _updateViewIfUndefined and _updateViewIfOutside into _updateKeyboardFocus. It's completely untested and you'll obviously need to tweak the other code, but here it is in case you want to give it a try:

_updateKeyboardFocus(origEl, nextId, offset) {
  let nextDate;
  if (!this.state.days[nextId]) {
    let origId = origEl.dataset.id;
    nextDate = this.state.days[origId].dateObj;
    // Get the date that needs to be focused next:
    nextDate.setDate(nextDate.getDate() + offset);
  } else if (this.elements.daysView[nextId].classList.contains("outside")) {
    nextDate = this.state.days[nextId].dateObj;
  }
  // Update the month view to include this date:
  this.state.setMonthByOffset(Math.sign(offset));
  // Find the "data-id" of the date element:
  nextId = this._calculateNextId(nextDate);

  // Now that we have the date, focus it.
  const nextEl = this.elements.daysView[nextId];
  origEl.removeAttribute("tabindex");
  nextEl.setAttribute("tabindex", "0");
  nextEl.focus();
},

origEl will be event.target.

Adds focusedDate into the Calendar state object to track and update focusable elements for the grid when a dateView is rendered and especially handle keyboard events. The calculation of the next focused date is improved and delegated to the combination of the dateKeeper's setCalendarMonth method and vanilla JavaScript Date object methods.

This patch also refactors the logic for updating the grid based on the different states of the next focused day (i.e. when it is a day from another month, when it is the same day of another month, or the first of the month). This resolves the Page Up/Page Down related bug 1806645 as well.

TODO: Correct focus placement when Previous/Next Month buttons are used. This will be another patch in this stack

Attachment #9312727 - Attachment description: WIP: Bug 1803608 - Update calendar code to handle key events using dateKeeper. r?mconley,kcochrane,Jamie → Bug 1803608 - Update calendar code to handle key events using dateKeeper. r?mconley,kcochrane,Jamie
Pushed by ayeddi@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/37a00f91d082 Update calendar code to handle key events using dateKeeper. r=mconley,kcochrane
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 111 Branch

Comment on attachment 9312727 [details]
Bug 1803608 - Update calendar code to handle key events using dateKeeper. r?mconley,kcochrane,Jamie

Beta/Release Uplift Approval Request

  • User impact if declined: The keyboard accessibility allows users with screen readers and users with limited mobility access the functionality of this component - a date picker calendar for date and datetime-local inputs only. The patch ensures that the calendar grid itself still consistently provides a focusable day for a keyboard user to get to the calendar grid and be able to navigate between days to make a selection. With this patch, the keyboard focusability logic will be handled by the general dateKeeper component instead of a custom function that, in some cases, triggered buggy behavior (see bugs 1806645 and 1806823)
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: (included in the bug description)
  • List of other uplifts needed: Bug 1806645, Bug 1806823
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The changes only impact keyboard accessibility for one component only (a date picker calendar for date and datetime-local inputs only), it does not change any looks and no other aspects of the feel of these components
  • Low risk improvements early in the cycle (the keyboard accessibility was only added in 109)
  • Enabling features that have been validated/tested by QA (the fix for this bug and for its parent in the stack, the bug 1806645, have been tested and confirmed by the QA team)
  • String changes made/needed: none
  • Is Android affected?: No
Attachment #9312727 - Flags: approval-mozilla-beta?

Comment on attachment 9312727 [details]
Bug 1803608 - Update calendar code to handle key events using dateKeeper. r?mconley,kcochrane,Jamie

Approved for 110 beta 5, thanks.

Attachment #9312727 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Blocks: 1806823
Blocks: 1806645
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: