Conversation
WalkthroughThis update replaces hardcoded English Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant TwigTemplate
participant Translator
User->>Browser: Loads a page
Browser->>TwigTemplate: Renders template
TwigTemplate->>Translator: Requests translation for aria-label/alt key
Translator-->>TwigTemplate: Returns localized string
TwigTemplate-->>Browser: Outputs HTML with localized aria-label/alt
Browser-->>User: Displays accessible, localized UI
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ Preview Environment deleted from BunnyshellAvailable commands:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/Sylius/Bundle/UiBundle/Resources/translations/messages.en.yml (3)
21-23: Check naming consistency for the new “account_*” keys
account_button,account_navigation_desktop, andaccount_navigation_mobileare added with plain-English values.
Other UI strings for buttons (e.g.login_button,register_button) consistently end with_button.
Consider renamingaccount_buttontoaccount_navigation_button(or similar) to keep the namespace symmetrical, or at least verify no clash with existing keys in other locale files.
No functional change is required, just a small taxonomy tidy-up.
370-371: Footer link keys: be specific
footer_help_linksandfooter_informational_linksread like plural buckets rather than the text that will be voiced by screen-readers.
If the aria-use-case is “Help links” / “Informational links”, you may want the singular form (footer_help/footer_information) for clarity.
Not critical; purely wording finesse.
876-876: Duplication with existing “sylius_logo”
sylius_logo_ariais fine, but there is already asylius_logokey.
Consider appending a comment explaining the difference (visual text vs. aria-label) so future maintainers don’t consolidate them by mistake.src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/footer/content/copy.html.twig (1)
2-2: Localized logo aria-label looks correct
sylius.ui.sylius_logo_ariakey exists and the label will now be localised.
Remember to keep the underlying SVG/img’saltempty (alt="") to avoid double-speaking if the included logo template already carries descriptive alt text.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/Sylius/Bundle/ShopBundle/templates/homepage/banner.html.twig(1 hunks)src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/footer/content/copy.html.twig(1 hunks)src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/footer/content/menu.html.twig(2 hunks)src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/header/content/security/visitor/desktop.html.twig(1 hunks)src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/header/content/security/visitor/desktop/register.html.twig(1 hunks)src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/header/content/security/visitor/mobile.html.twig(1 hunks)src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/header/navbar/menu.html.twig(1 hunks)src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/metatags.html.twig(0 hunks)src/Sylius/Bundle/UiBundle/Resources/translations/messages.en.yml(5 hunks)
💤 Files with no reviewable changes (1)
- src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/metatags.html.twig
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Static checks / PHP 8.2, Symfony ^6.4
- GitHub Check: Static checks / PHP 8.3, Symfony ^7.2
- GitHub Check: Static checks / PHP 8.4, Symfony ^7.2
🔇 Additional comments (11)
src/Sylius/Bundle/UiBundle/Resources/translations/messages.en.yml (2)
151-151: Potential duplicate “close” keyA “close” label is common in modal components; double-check that this key does not already exist elsewhere in the same domain to avoid silent overwrite in YAML.
If an older key exists, drop one or merge to keep a single source of truth.
893-893: Confirm desktop / mobile symmetryYou added
taxon_navigation_desktop.
Is there a matchingtaxon_navigation_mobile?
Keeping the pair makes future template work easier.src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/header/content/security/visitor/desktop/register.html.twig (1)
10-10: ARIA label now localised – good jobThe extra attribute
{ 'role': 'menuitem', 'aria-label': 'sylius.ui.register'|trans }cleanly localises the label.
Just verify thatbuttons.transparentdoes not already set anaria-label; duplicate attributes will be ignored by browsers but lint-tools may warn.src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/header/content/security/visitor/mobile.html.twig (1)
1-3: Mobile account navigation ARIA labels localisedBoth the container and the icon button now pull translated labels – accessibility & i18n improved.
No further issues spotted.src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/header/content/security/visitor/desktop.html.twig (1)
1-1: Desktop account navigation label localisedGood conversion to translation key – improves accessibility without side-effects.
src/Sylius/Bundle/ShopBundle/templates/homepage/banner.html.twig (2)
4-4: Looks good: Thealtattribute for the main banner image is now translatable, matching the original “Home” text.
5-5: Looks good: Thealttext for the secondary image is now localized viasylius.homepage.new_collection.src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/header/navbar/menu.html.twig (2)
3-3: Looks good: Navigation landmark’saria-labelis now translatable withsylius.ui.taxon_navigation_desktop.
8-8: Looks good: Close button’saria-labeluses thesylius.ui.closekey for localization.src/Sylius/Bundle/ShopBundle/templates/shared/layout/base/footer/content/menu.html.twig (2)
4-4: Looks good: Footer informational links nav is now labelled viasylius.ui.footer_informational_linksfor accessibility.
13-13: Looks good: Footer help links nav usessylius.ui.footer_help_linksto localize thearia-label.
| Q | A |-----------------|----- | Branch? | 2.1 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | – | License | MIT ### Summary Fix four obvious typos in `CHANGELOG-2.1.md`: - `Uploud` → `Upload` (v2.1.5, PR #18286) - `confflict` → `conflict` (v2.1.5, PR #18302) - `nessecary` → `necessary` (v2.1.5, PR #18342) - `lables` → `labels` (v2.1.2, PR #18180) ### Notes - Minimal diffs only; no punctuation/style changes. - Code, identifiers, and formatting are untouched. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Corrected typos in changelog entries for improved readability and clarity. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit