Conversation
Signed-off-by: James Din <1267380+jamesDin@users.noreply.github.com>
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
Author
|
Please approve the Hebrew changes I've added. Thanks :-) |
Member
|
Thank you for the contribution. 🙏 |
Signed-off-by: James Din <1267380+jamesDin@users.noreply.github.com>
Signed-off-by: James Din <1267380+jamesDin@users.noreply.github.com>
Updated missingKeysCount for Hebrew localization. Signed-off-by: James Din <1267380+jamesDin@users.noreply.github.com>
Author
|
Ok, please check now :-) |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Fix the recurrence weekday-format handling before approval.
Pull request overview
Adds Hebrew (he-IL) localization support for Scheduler components.
Changes:
- Adds Hebrew translations and localized formatting helpers.
- Updates localization metadata to mark Hebrew as complete.
- Recurrence weekday formatting requires correction for Day.js and Moment adapters.
File summaries
| File | Summary |
|---|---|
packages/x-scheduler/src/locales/heIL.ts |
Hebrew translations and formatting callbacks; recurrence weekday formatting issue remains. |
docs/data/scheduler/localization/data.json |
Updates Hebrew localization completeness metadata. |
Review details
Suppressed comments (1)
packages/x-scheduler/src/locales/heIL.ts:120
- These callbacks receive
weekdayShortfromRecurrenceTab, where it is produced withadapter.formatByString(monthlyRef.date, 'ccc'). The Day.js and Moment adapters pass that format directly to their libraries, whose weekday tokens aredd/ddd/dddd, so the value can be the literalccc;localizeWeekdaythen falls through and the Hebrew monthly recurrence label becomesccc .... Please use the adapter'sweekdayShortformat (or otherwise normalize the value per adapter) before calling this locale callback.
recurrenceMonthlyLastWeekAriaLabel: (weekDay) => monthlyWeekday(-1, weekDay),
recurrenceMonthlyLastWeekLabel: (weekDay) => monthlyWeekday(-1, weekDay, true),
recurrenceMonthlyPresetLabel: (dayNumber) => `ב־${dayNumber} בכל חודש`,
recurrenceMonthlyWeekNumberAriaLabel: (ord, weekDay) => monthlyWeekday(ord, weekDay),
recurrenceMonthlyWeekNumberLabel: (ord, weekDay) => monthlyWeekday(ord, weekDay, true),
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
LukasTy
reviewed
Sep 14, 2026
LukasTy
left a comment
Member
There was a problem hiding this comment.
Claude Opus review
Some nitpicks/cleanups.
The translations look good and CI is green. Fix one type issue before merge.
heIL.ts:74,146,220-- KeepPartial<...>onheILDialog,heILCalendar, andheILTimeline. Every other non-English locale in every package usesPartial. With the full type, the next key that someone adds toEventEditingLocaleTextorEventCalendarLocaleTextbreakspnpm typescriptinheIL.ts, becausepnpm l10nadds the new key only as a comment.missingKeysCountalready tracks completeness.heIL.ts:113--weekday ?? weekdayName:weekdayis a requiredSchedulerWeekday, so the fallback never runs. UselocalizeWeekday(weekday).- Follow-up, not blocking:
localizeWeekdayaccepts four input shapes becauseRecurrenceTab.tsx:430-436passes the English token ('monday') to the monthly...AriaLabelcallbacks but a date-localecccstring to the...Labelcallbacks. English screen readers announce "monday of the last week of the month". If the callbacks always get theSchedulerWeekdaytoken (or{ weekday, weekdayName }, likerecurrenceWeeklyPresetLabel), locales need only a static map.
@rita-codes, could you check if this ☝️ is feasible?
Member
Updated locale definitions to use Partial types for EventEditingLocaleText. Signed-off-by: James Din <1267380+jamesDin@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.