Skip to content

feat: add showMultiDayTimes property to calendar component#2204

Open
iobuhov wants to merge 7 commits into
mainfrom
feat/showMultiDayTimes_Property-migrated
Open

feat: add showMultiDayTimes property to calendar component#2204
iobuhov wants to merge 7 commits into
mainfrom
feat/showMultiDayTimes_Property-migrated

Conversation

@iobuhov

@iobuhov iobuhov commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR adds the showMultiDayTimes property to the calendar component.

Migrated from #2121 (cherry-picked onto fresh main branch to resolve conflicts).

Original PR

#2121

🤖 Generated with Claude Code

@github-actions

This comment has been minimized.

samuelreichert
samuelreichert previously approved these changes May 7, 2026
@github-actions

This comment has been minimized.

@gjulivan

Copy link
Copy Markdown
Collaborator

gjulivan
gjulivan previously approved these changes Jul 14, 2026
leonardomendix
leonardomendix previously approved these changes Jul 22, 2026
@iobuhov
iobuhov dismissed stale reviews from leonardomendix and gjulivan via 5840800 July 23, 2026 08:05
@iobuhov
iobuhov force-pushed the feat/showMultiDayTimes_Property-migrated branch from 263dd20 to 5840800 Compare July 23, 2026 08:05
@github-actions

This comment has been minimized.

gjulivan
gjulivan previously approved these changes Jul 23, 2026
iobuhov and others added 3 commits July 23, 2026 11:18
…e in specs

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…po installs

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge


What was reviewed

File Change
packages/pluggableWidgets/calendar-web/src/Calendar.xml Added showMultiDayTimes boolean property (default false)
packages/pluggableWidgets/calendar-web/typings/CalendarProps.d.ts Added showMultiDayTimes: boolean to container and preview props
packages/pluggableWidgets/calendar-web/src/helpers/CalendarPropsBuilder.ts Passed showMultiDayTimes through and added eventTimeRangeStartFormat / eventTimeRangeEndFormat
packages/pluggableWidgets/calendar-web/src/__tests__/Calendar.spec.tsx Added tests for pass-through, start/end formats, and showEventDate interaction
packages/pluggableWidgets/calendar-web/src/__tests__/__snapshots__/Calendar.spec.tsx.snap Updated snapshot with data-show-multi-day-times, data-min, data-max attributes
packages/pluggableWidgets/calendar-web/CHANGELOG.md Added [Unreleased] entry for new property
pnpm-workspace.yaml Added recursiveInstall: true, reformatted with consistent indentation
packages/pluggableWidgets/calendar-web/openspec/changes/add-show-multi-day-times/ OpenSpec artifacts (proposal, spec, tasks)

Skipped (out of scope): pnpm-lock.yaml


Findings

⚠️ Low — eventTimeRangeStartFormat / eventTimeRangeEndFormat always set regardless of showMultiDayTimes

File: packages/pluggableWidgets/calendar-web/src/helpers/CalendarPropsBuilder.ts lines 169–178
Note: eventTimeRangeStartFormat and eventTimeRangeEndFormat are set whenever a timeFormat is configured, independently of whether showMultiDayTimes is true or false. This is likely intentional — react-big-calendar only uses these formats when showMultiDayTimes is enabled — but it is worth confirming. If the library uses them for other purposes when showMultiDayTimes=false, unexpected time labels could appear on events. A comment confirming the intent (or gating behind showMultiDayTimes) would prevent future confusion.


⚠️ Low — pnpm-workspace.yaml adds recursiveInstall: true without explanation

File: pnpm-workspace.yaml line 1
Note: Adding recursiveInstall: true changes workspace install behaviour for all packages in the monorepo. This is a cross-cutting side-effect unrelated to the feature being implemented. If it was intentional (e.g. to fix an install issue found during development), a brief commit note or PR comment explaining why would help reviewers assess the risk. If it was unintentional, it should be reverted.


Positives

  • The default false correctly preserves existing all-day row behaviour for multi-day events — zero-risk for existing consumers.
  • showEventDate=false correctly blanks all three time-range formats (eventTimeRangeFormat, eventTimeRangeStartFormat, eventTimeRangeEndFormat) atomically at the end of buildFormats, which prevents any ordering dependency.
  • Test suite covers all four scenarios from the spec: pass-through flag, start/end format content and separator, pattern consistency across the three format functions, no formats when timeFormat is unconfigured, blanking on showEventDate=false, and preservation on showEventDate=true. Thorough and easy to read.
  • CalendarProps.d.ts is updated in both CalendarContainerProps and CalendarPreviewProps — no XML/TS drift.
  • XML property key is lowerCamelCase (showMultiDayTimes) and matches the TypeScript prop exactly — consistent with repo conventions.
  • CHANGELOG entry is user-facing and describes observable behaviour, not implementation details — matches repo convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants