Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 850b6e6439
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/styles/calendar.scss
Outdated
| .forward, | ||
| .back { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
Hiding both pagination buttons in mobile mode removes the only control that changes months by offset; the new caption picker only changes the year for the current month slot. With the default mobile view (e.g., two rendered months), users cannot navigate to other months in the same year (such as moving from April/May to September), which blocks valid date selections on mobile.
Useful? React with 👍 / 👎.
src/calendar.js
Outdated
| if (trigger) { | ||
| const monthElement = trigger.closest('.js-month'); | ||
|
|
||
| if (this.activeYearPicker === monthElement) { | ||
| this.closeYearPicker(); |
There was a problem hiding this comment.
The document-level click handler reacts to any data-year-picker-trigger in the page without checking that it belongs to this.el. On pages rendering multiple calendar widgets, clicking one widget’s caption trigger causes other instances to treat that external month node as active, so their picker state and subsequent year selections can mutate the wrong calendar instance.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57e9047be9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/config.js
Outdated
| displayMonths: 2, | ||
| displayMonthsMobile: null, // falls back to displayMonths | ||
| mobileBreakpoint: 767, // px, <= breakpoint is mobile | ||
| showPaginationMobile: false, |
There was a problem hiding this comment.
Setting showPaginationMobile to false by default introduces a backward-incompatible regression: mobile mode now hides both pagination arrows unless consumers explicitly opt in, so existing embeds lose month navigation on mobile after upgrading. This is especially user-visible when displayMonthsMobile is 1, where users cannot move beyond the initial month anymore.
Useful? React with 👍 / 👎.
| const { | ||
| year: yearStart, | ||
| month: monthStart | ||
| } = this.shiftMonth(year, month, -monthElement.slotIndex); |
There was a problem hiding this comment.
renderFromSlot shifts the selected month backward by slotIndex without enforcing minimumSelectableYear(). When a user picks the minimum year from a non-zero slot in an early month (for example January in slot 1), the computed start rolls into the previous year, so months earlier than the intended minimum become visible despite the year-grid bound.
Useful? React with 👍 / 👎.
…eader support Add arrow key grid navigation (Left/Right/Up/Down/Home/End/PageUp/PageDown), visible focus indicators on day cells, aria-disabled on unavailable dates, aria-current on today and selected year, enriched aria-labels with availability info, aria-live region for selection announcements, tooltip aria-describedby linking, dialog aria-label, prefers-reduced-motion support, and localized a11y strings across all 10 locales. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…trap Arrow keys navigate the 3-column year grid, focus moves to the selected year on open and returns to the trigger on close, Tab/Shift+Tab are trapped within the panel, and dialog semantics (role, aria-modal) are applied when the picker is visible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6b4a5fc5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/calendar.js
Outdated
| if (yearStart < minimumYear) { | ||
| yearStart = minimumYear; | ||
| monthStart = 0; | ||
| } |
There was a problem hiding this comment.
renderFromSlot only clamps on yearStart < minimumYear, so selecting the current year from a future month slot can still produce a start month earlier than the current month. For example, after paging forward to January 2027 and choosing 2026 while currentDate is April 2026, this path renders January 2026, which bypasses the default isBackDisabled restriction and exposes past months that pagination blocks.
Useful? React with 👍 / 👎.
| if (trigger) { | ||
| trigger.setAttribute('aria-expanded', 'false'); | ||
| trigger.focus(); | ||
| } |
There was a problem hiding this comment.
closeYearPicker always focuses the caption trigger, including when the picker is dismissed by clicking outside the calendar. In that path, users who click an external control (especially text inputs) can lose focus back to the calendar trigger, forcing an extra click and breaking expected interaction flow. Trigger refocus should be limited to keyboard-initiated dismissals.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 247f63815d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| minimumSelectableYear() { | ||
| return this.opts.currentDate[0]; | ||
| } |
There was a problem hiding this comment.
minimumSelectableYear() always returns currentDate[0], so the year picker clamps to the current year even when the calendar is explicitly configured to allow past navigation (for example isBackDisabled: false or enableAllDays: true). Because both renderYearGrid() and renderFromSlot() rely on this bound, users can page back to earlier years with arrows but cannot use the year picker to view or select those same years, creating a functional mismatch in configured historical-navigation flows.
Useful? React with 👍 / 👎.
src/calendar.js
Outdated
| newCells.forEach((c) => { | ||
| if (c.getAttribute('data-value') === dayValue) { | ||
| target = c; | ||
| } |
There was a problem hiding this comment.
After month paging, the focus restore loop keeps assigning target for every matching day number and never stops at the first match. In multi-month views this means keys like PageDown often move focus to the last rendered month (since day numbers repeat across months) instead of the corresponding day in the originating slot, which breaks predictable keyboard navigation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bf718c1f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (yearOption.parent && yearOption.value) { | ||
| const selectedYear = parseInt(yearOption.value, 10); | ||
| const monthElement = this.activeYearPicker; | ||
| this.closeYearPicker(); |
There was a problem hiding this comment.
Selecting a year via keyboard (Enter/Space on a data-year-option button) closes the picker without restoring focus, and the focused control then becomes hidden while the calendar rerenders. In this path, focus typically falls back to body, leaving keyboard users without a predictable next focus target after changing year. Please preserve keyboard focus by restoring it to the originating caption trigger (or equivalent in-slot control) when the selection is keyboard-initiated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78e2381cd5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "babel-loader": "^10.1.1", | ||
| "chai": "^6.2.2", | ||
| "css-loader": "^7.1.4", | ||
| "eslint": "^10.2.0", |
There was a problem hiding this comment.
package.json still declares "node": ">=18.12.0", but this upgrade bumps core dev tooling to versions that require Node 20+ (for example, eslint@10.2.0 and yargs@18.0.0, which the updated lockfile resolves with ^20.19.0 minimum). On environments that follow the declared engine (Node 18), installs or CI checks can now fail (especially with engine-strict) even though the project claims Node 18 compatibility. Please either pin Node-18-compatible tool versions or raise the project’s engines.node floor accordingly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fb4e8e3cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/calendar.js
Outdated
| this.dom.tooltip.id = 'calendar-tooltip'; | ||
| this.dom.tooltip.querySelector('span').innerHTML = invalidRangeMessage; | ||
| cell.setAttribute('aria-describedby', 'calendar-tooltip'); |
There was a problem hiding this comment.
The tooltip ID is hard-coded to calendar-tooltip, so pages with multiple calendar widgets can end up with duplicate IDs and ambiguous aria-describedby targets. In that scenario, assistive tech may announce the wrong invalid-range message because the cell can resolve to another widget’s tooltip. Use an instance-scoped ID (for example, include a per-calendar UID) when setting both the tooltip id and the cell’s aria-describedby.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d153670a37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (target) { | ||
| target.focus(); | ||
| } |
There was a problem hiding this comment.
In keyboard range selection mode, the PageUp/PageDown branch only restores focus after paging and never calls mouseoverHandler (unlike Arrow/Home/End). That leaves hasValidRange and highlighted bounds stale from the previous cell, so pressing Enter can complete a range that was never revalidated for min/max stay or blocked-day rules after the month jump.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
No description provided.