fix: future hours translations #995
Conversation
|
| Key | Languages Removed |
|---|---|
closesAt |
cs,da de,en en-GB,es et,fi fr,hr hu,it ja,lt lv,nb nl,pl pt,ro sk,sv tr,zh zh-TW |
opensAt
| Key | Languages Removed |
|---|---|
opensAt |
cs,da de,en en-GB,es et,fi fr,hr hu,it ja,lt lv,nb nl,pl pt,ro sk,sv tr,zh zh-TW |
|
Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information. |
WalkthroughThis PR replaces Sequence Diagram(s)sequenceDiagram
participant UI as Client/UI
participant HS as HoursStatus component
participant TM as ThemeManager
participant I18n as i18n
UI->>HS: render with business hours props
HS->>TM: compute classNameResolved
HS->>I18n: request localized templates (closesAtTime / opensAtTime / *Week)
HS->>HS: statusTemplate(params) -> determine isOpen24h / isIndefinitelyClosed / isFuture
HS->>HS: format time and optional dayOfWeek
HS->>UI: return rendered node with resolved className and localized status text
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/visual-editor/src/components/atoms/hoursStatus.tsx (1)
181-187: ReplaceHoursStatusParamswith the importedStatusParamstype to fix type inconsistency.The local
HoursStatusParamsinterface duplicates the structure of the importedStatusParamsfrom@yext/pages-componentsbut weakens type safety by usinganyfor the interval objects. Additionally, the helper functionsisOpen24h,isIndefinitelyClosed, anddefaultSeparatorTemplate(lines 167, 171, 175) are typed to acceptStatusParams, but they receiveHoursStatusParamswhen called on lines 52, 67, and 118—creating a type mismatch in strict mode.Use the imported
StatusParamsdirectly:Changes needed
Remove the local interface:
-interface HoursStatusParams { - isOpen: boolean; - currentInterval: any | null; - futureInterval: any | null; - timeOptions?: Intl.DateTimeFormatOptions; - dayOptions?: Intl.DateTimeFormatOptions; -}Update the
statusTemplateparameter type on line 51:- statusTemplate={(params: HoursStatusParams) => { + statusTemplate={(params: StatusParams) => {Update the
hoursCurrentTemplateOverridefunction signature on line 140:- params: HoursStatusParams, + params: StatusParams,
🤖 Fix all issues with AI agents
In `@packages/visual-editor/locales/components/es/visual-editor.json`:
- Around line 46-47: The translation key opensAtTime uses the wrong placeholder
name ({{hora}}) which does not match the renderer's expected {{time}}; update
the value of the opensAtTime entry to use {{time}} instead of {{hora}} so it
matches opensAtTimeWeek and the rendering code that expects {{time}}.
- Around line 14-15: The Spanish translation key "closesAtTime" uses the wrong
placeholder "{{hora}}", which prevents the renderer from interpolating the time;
update the value for the "closesAtTime" entry to use the expected "{{time}}"
placeholder (matching "closesAtTimeWeek" which already uses "{{time}}") so
interpolation works correctly.
In `@packages/visual-editor/locales/components/pt/visual-editor.json`:
- Around line 14-15: The Portuguese locale entries use the wrong interpolation
token: update the keys "closesAtTime" and "opensAtTime" (and the duplicate
entries later in the file) to use the {{time}} placeholder instead of {{hora}}
so they match the rest of the templates and the rendering code that expects
{{time}}.
In `@packages/visual-editor/locales/platform/es/visual-editor.json`:
- Around line 66-67: The translation key "closesAtTime" uses the wrong
placeholder "{{hora}}" which doesn't match the renderer's expected "{{time}}";
update the value for the "closesAtTime" key to use "{{time}}" (consistent with
"closesAtTimeWeek") so the time interpolates correctly at runtime.
- Around line 590-591: The translation key "opensAtTime" uses the wrong
placeholder "{{hora}}" which doesn't match the renderer's expected "{{time}}";
update the value for the "opensAtTime" key to use "{{time}}" (consistent with
"opensAtTimeWeek") so the time renders correctly.
In `@packages/visual-editor/locales/platform/pt/visual-editor.json`:
- Around line 66-67: The translation key "closesAtTime" uses the wrong
placeholder {{hora}} which prevents interpolation; update the value for the
"closesAtTime" entry to use the expected {{time}} placeholder so it matches the
rest of the locale keys (e.g., "closesAtTimeWeek") and the renderer that
replaces {{time}}.
- Around line 591-592: The localization key opensAtTime uses the wrong
placeholder ({{hora}}) while the renderer expects {{time}}; update the value for
opensAtTime to use the {{time}} placeholder to match opensAtTimeWeek and the
rendering code so the time is displayed correctly (i.e., change "Abre às
{{hora}}" to use "{{time}}").
In `@packages/visual-editor/src/components/atoms/hoursStatus.tsx`:
- Around line 51-52: The isFuture boolean is inverted: it currently uses
isOpen24h(params) || isIndefinitelyClosed(params) which makes it true when there
is no future time; change the logic in the statusTemplate so isFuture reflects
that a future open/close time exists (e.g., isFuture = !(isOpen24h(params) ||
isIndefinitelyClosed(params)) or equivalently isFuture = !isOpen24h(params) &&
!isIndefinitelyClosed(params)) so the subsequent statusText branch (in
statusTemplate) runs for normal open/close cases instead of 24h/indefinitely
closed cases.
🟡 Minor comments (18)
packages/visual-editor/locales/components/nl/visual-editor.json-14-15 (1)
14-15: Tweak Dutch day-of-week phrasing for clarity.“Sluit om {{time}} {{dayOfWeek}}” / “Opent om {{time}} {{dayOfWeek}}” reads unnatural in Dutch. Consider “Sluit om {{time}} op {{dayOfWeek}}” or “Sluit op {{dayOfWeek}} om {{time}}” (same for “Opent …”).
Also applies to: 46-47
packages/visual-editor/locales/components/cs/visual-editor.json-14-15 (1)
14-15: Fix Czech word order/preposition for day-of-week variants.As written, the strings render as “Zavírá v 17:00 pátek” / “Otevírá v 17:00 pátek,” which is ungrammatical. Prefer placing the day before the time (or adding “v” before the day).
💡 Suggested update
- "closesAtTimeWeek": "Zavírá v {{time}} {{dayOfWeek}}", + "closesAtTimeWeek": "Zavírá {{dayOfWeek}} v {{time}}", ... - "opensAtTimeWeek": "Otevírá v {{time}} {{dayOfWeek}}", + "opensAtTimeWeek": "Otevírá {{dayOfWeek}} v {{time}}",Also applies to: 56-57
packages/visual-editor/locales/components/fi/visual-editor.json-14-15 (1)
14-15: Inconsistent use of "klo" in Finnish time expressions.
closesAtTimeWeek(line 15) includes "klo" before the time placeholder, but the other three strings (closesAtTime,opensAtTime,opensAtTimeWeek) omit it. In Finnish, "klo" is the standard way to express clock time (equivalent to "at" for times in English).For consistency, either add "klo" to the other three strings or remove it from
closesAtTimeWeek:Option 1: Add "klo" to all (recommended for proper Finnish)
- "closesAtTime": "Suljetaan {{time}}", - "closesAtTimeWeek": "Suljetaan klo {{time}} {{dayOfWeek}}", + "closesAtTime": "Suljetaan klo {{time}}", + "closesAtTimeWeek": "Suljetaan klo {{time}} {{dayOfWeek}}",- "opensAtTime": "Avautuu {{time}}", - "opensAtTimeWeek": "Avautuu {{time}} {{dayOfWeek}}", + "opensAtTime": "Avautuu klo {{time}}", + "opensAtTimeWeek": "Avautuu klo {{time}} {{dayOfWeek}}",Option 2: Remove "klo" from closesAtTimeWeek for consistency
- "closesAtTimeWeek": "Suljetaan klo {{time}} {{dayOfWeek}}", + "closesAtTimeWeek": "Suljetaan {{time}} {{dayOfWeek}}",Also applies to: 46-47
packages/visual-editor/locales/platform/zh/visual-editor.json-586-587 (1)
586-587: Consider “{{dayOfWeek}} {{time}}” order and more natural label.“开放时间” is understandable, but “营业时间” is more idiomatic. Also, day-of-week typically comes before time.
✏️ Suggested wording
- "opensAtTime": "开放时间：{{time}}", - "opensAtTimeWeek": "开放时间：{{time}} {{dayOfWeek}}", + "opensAtTime": "营业时间：{{time}}", + "opensAtTimeWeek": "营业时间：{{dayOfWeek}} {{time}}",packages/visual-editor/locales/platform/zh/visual-editor.json-66-67 (1)
66-67: Prefer day-of-week before time for Chinese phrasing.Current order reads a bit unnatural in Chinese. Suggest swapping placeholders to “{{dayOfWeek}} {{time}}”.
✏️ Suggested wording
- "closesAtTimeWeek": "于 {{time}} {{dayOfWeek}} 关闭", + "closesAtTimeWeek": "于 {{dayOfWeek}} {{time}} 关闭",packages/visual-editor/locales/components/et/visual-editor.json-46-47 (1)
46-47: Apply consistent "kell" usage for opening hours as well.Same fix needed here for
opensAtTimeto matchopensAtTimeWeek.packages/visual-editor/locales/components/et/visual-editor.json-14-15 (1)
14-15: Same "kell" inconsistency applies here.Apply the same fix as noted in the platform locale file to maintain consistency between
closesAtTimeandclosesAtTimeWeek.packages/visual-editor/locales/platform/et/visual-editor.json-66-67 (1)
66-67: Minor inconsistency: "kell" usage differs between time-only and time-with-day variants.The
closesAtTimetranslation omits "kell" ("Suletakse {{time}}"), whileclosesAtTimeWeekincludes it ("Suletakse kell {{time}} {{dayOfWeek}}"). For grammatical consistency in Estonian, consider either adding "kell" to the time-only variant or removing it from the time-with-day variant.Suggested fix (add "kell" for consistency)
- "closesAtTime": "Suletakse {{time}}", + "closesAtTime": "Suletakse kell {{time}}",packages/visual-editor/locales/platform/et/visual-editor.json-591-592 (1)
591-592: Same inconsistency with "kell" usage in opening hours translations.Similar to the closing hours keys,
opensAtTimeomits "kell" whileopensAtTimeWeekincludes it. Apply the same fix for consistency.Suggested fix
- "opensAtTime": "Avatakse {{time}}", + "opensAtTime": "Avatakse kell {{time}}",packages/visual-editor/locales/platform/ja/visual-editor.json-586-587 (1)
586-587: Same spacing issue in the open-week template.There’s also an extra space before 「に」 here, which will render as a visible gap.
💡 Suggested fix
- "opensAtTimeWeek": "{{time}} {{dayOfWeek}} に開店", + "opensAtTimeWeek": "{{time}} {{dayOfWeek}}に開店",packages/visual-editor/locales/platform/ja/visual-editor.json-66-67 (1)
66-67: Remove the extra space before 「に」 in the week template.The current string will render with an unintended gap before the particle. Consider removing the space before 「に」 to avoid awkward spacing.
💡 Suggested fix
- "closesAtTimeWeek": "{{time}} {{dayOfWeek}} に閉店", + "closesAtTimeWeek": "{{time}} {{dayOfWeek}}に閉店",packages/visual-editor/locales/components/tr/visual-editor.json-46-47 (1)
46-47: Avoid fixed locative suffix on{{dayOfWeek}}In Turkish, the locative suffix varies (-da/-de/-ta/-te), so hardcoding
'damakes outputs like “Pazartesi'da” incorrect. Consider rephrasing to avoid suffixing the day name.✅ Suggested localization tweak
- "opensAtTimeWeek": "{{time}} {{dayOfWeek}}'da açılıyor", + "opensAtTimeWeek": "{{dayOfWeek}} günü {{time}}'da açılıyor",packages/visual-editor/locales/components/sk/visual-editor.json-14-15 (1)
14-15: Add Slovak preposition for day-of-week expressions."Zatvára sa o {{time}} {{dayOfWeek}}" and "Otvára sa o {{time}} {{dayOfWeek}}" are missing the required Slovak preposition "v" (or "vo" before certain consonant clusters). These should read "v {{dayOfWeek}}" to be grammatically correct in Slovak. Additionally, verify that the {{dayOfWeek}} placeholder is replaced with lowercase day names (e.g., "v pondelok", not "v Pondelok").
Suggested fix
- "closesAtTimeWeek": "Zatvára sa o {{time}} {{dayOfWeek}}", + "closesAtTimeWeek": "Zatvára sa o {{time}} v {{dayOfWeek}}",- "opensAtTimeWeek": "Otvára sa o {{time}} {{dayOfWeek}}", + "opensAtTimeWeek": "Otvára sa o {{time}} v {{dayOfWeek}}",packages/visual-editor/locales/platform/tr/visual-editor.json-591-592 (1)
591-592: Reorder day-of-week and time for natural Turkish phrasing.The current phrasing places time before day-of-week and uses an unusual locative suffix on the day name. Turkish grammar places the day-of-week before time for natural expression, and uses "günü" (the standard form meaning "on [day]") rather than the locative suffix. The suggested change produces more grammatically correct and natural Turkish.
🔧 Suggested wording
- "opensAtTimeWeek": "{{time}} {{dayOfWeek}}'da açılıyor", + "opensAtTimeWeek": "{{dayOfWeek}} günü {{time}}'da açılıyor",packages/visual-editor/locales/platform/tr/visual-editor.json-66-67 (1)
66-67: Reorder day-of-week and time for natural Turkish phrasing.Turkish grammar requires the day-of-week to precede the time. Update both
closesAtTimeWeek(line 67) andopensAtTimeWeek(line 592) to follow Turkish word order conventions.🔧 Suggested fixes
- "closesAtTimeWeek": "{{time}} {{dayOfWeek}} itibarıyla kapanıyor", + "closesAtTimeWeek": "{{dayOfWeek}} günü {{time}} itibarıyla kapanıyor",- "opensAtTimeWeek": "{{time}} {{dayOfWeek}}'da açılıyor", + "opensAtTimeWeek": "{{dayOfWeek}} günü {{time}}'da açılıyor",packages/visual-editor/locales/components/lv/visual-editor.json-51-52 (1)
51-52: Correct the Latvian day-of-week grammatical case in both opening and closing templates.The
{{dayOfWeek}}placeholder uses Intl.DateTimeFormat's nominative form (e.g., "Pirmdiena"), but Latvian grammar requires the stem form (e.g., "pirmdien") for adverbial time expressions like "Opens at {{time}} {{dayOfWeek}}". This applies to bothopensAtTimeWeekandclosesAtTimeWeektemplates. Consider post-processing the day-of-week string to convert it to the correct grammatical case or provide locale-specific stem forms.packages/visual-editor/locales/components/lv/visual-editor.json-14-15 (1)
14-15: Fix Latvian day-of-week grammar in closing/opening time templates.The current template appends day-of-week in nominative form (e.g., "Pirmdiena"), creating ungrammatical sentences in Latvian. Temporal expressions like "Opens at 10:00 Monday" require the adverbial form ("pirmdien") or locative case ("pirmdienā"), not nominative. The JavaScript
Intl.DateTimeFormatused inhoursStatus.tsxreturns nominative by default, which doesn't work for Latvian grammar.Either add a Latvian-specific transformation in the component to convert nominative to adverbial day forms, or create separate locale keys for days in adverbial form and substitute them when building the opening/closing time strings. See lines 14–15 and 51–52 in the locale file.
packages/visual-editor/locales/platform/pl/visual-editor.json-66-67 (1)
66-67: Polish verb form inconsistency: "opens" uses passive form while "closes" uses active reflexive."Otwarte o {{time}}" uses a passive/adjectival form, whereas "Zamyka się o {{time}}" uses the active reflexive form. For consistency with the reflexive active structure throughout (and matching other Slavic languages like Slovak and Czech which use parallel active verb forms), consider "Otwiera się o {{time}}" instead. Similarly for the week variants.
Suggested changes
- "opensAtTime": "Otwarte o {{time}}", - "opensAtTimeWeek": "Otwarte o {{time}} {{dayOfWeek}}", + "opensAtTime": "Otwiera się o {{time}}", + "opensAtTimeWeek": "Otwiera się o {{time}} {{dayOfWeek}}",Also applies to: 602-603
🧹 Nitpick comments (2)
packages/visual-editor/locales/platform/ro/visual-editor.json (2)
66-67: Consider flipping the weekday/time order for the week variant.“Se închide la {{time}} {{dayOfWeek}}” reads a bit awkwardly; Romanian typically places the weekday first (e.g., “luni la 17:00”). Suggest reordering the placeholders for the week variant.
🔧 Proposed tweak
- "closesAtTimeWeek": "Se închide la {{time}} {{dayOfWeek}}", + "closesAtTimeWeek": "Se închide {{dayOfWeek}} la {{time}}",
596-597: Same weekday/time ordering concern for the opens variant.Consider reordering to “{{dayOfWeek}} la {{time}}” for a more natural Romanian phrasing.
🔧 Proposed tweak
- "opensAtTimeWeek": "Se deschide la {{time}} {{dayOfWeek}}", + "opensAtTimeWeek": "Se deschide {{dayOfWeek}} la {{time}}",
No description provided.