Conversation
WalkthroughThis pull request adds right-to-left (RTL) layout support. The changes modify twig layout files by importing an RTL macro and updating the Changes
Sequence Diagram(s)sequenceDiagram
participant T as Layout Template
participant R as RTL Macro
participant L as App Locale
T->>R: Call rtl.default(app.locale)
R->>T: Return RTL attribute if locale is RTL (ar, he, fa, ur, ps, ku)
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:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Sylius/Bundle/UiBundle/Resources/views/Macro/rtl.html.twig (1)
1-9: Twig macro looks good but consider supporting locale variantsThe RTL detection macro correctly identifies the main RTL languages (Arabic, Hebrew, Persian, Urdu, Pashto, and Kurdish). The implementation follows good practices by checking if the locale starts with any of these language codes.
One minor improvement would be to consider region-specific locale variants (like 'ar-SA', 'ar-EG', etc.) by using a more robust comparison or documentation to clarify that such variants are already supported by the
starts withcheck.src/Sylius/Bundle/UiBundle/Resources/private/sass/rtl.scss (1)
4-12: Text direction handling looks good but consider refining input selectorsThe input text alignment and direction settings correctly handle different input types, with special cases for numbers, emails, and translation fields.
This approach ensures that content is properly displayed in RTL contexts while preserving LTR for content that needs it (like numbers).
Consider consolidating these selectors for better maintainability:
-input:not(input[type="number"], input[type="email"], input[name*="translations"]), textarea { +input:not([type="number"]):not([type="email"]):not([name*="translations"]), +textarea:not([name*="translations"]) { unicode-bidi: bidi-override; text-align: right; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Sylius/Bundle/AdminBundle/Resources/views/Layout/layout.html.twig(2 hunks)src/Sylius/Bundle/ShopBundle/Resources/private/scss/rtl.scss(1 hunks)src/Sylius/Bundle/ShopBundle/Resources/private/scss/style.scss(1 hunks)src/Sylius/Bundle/ShopBundle/Resources/views/layout.html.twig(2 hunks)src/Sylius/Bundle/UiBundle/Resources/private/sass/main.scss(1 hunks)src/Sylius/Bundle/UiBundle/Resources/private/sass/rtl.scss(1 hunks)src/Sylius/Bundle/UiBundle/Resources/views/Macro/rtl.html.twig(1 hunks)
🔇 Additional comments (19)
src/Sylius/Bundle/UiBundle/Resources/private/sass/main.scss (1)
3-3: Implementation of RTL support looks good.The addition of the RTL stylesheet import is well-placed after the variables and filters imports, ensuring that RTL styles can properly override or extend the base styles.
src/Sylius/Bundle/ShopBundle/Resources/private/scss/style.scss (1)
2-2: RTL stylesheet import is correctly placed.The import is positioned after the theme import, which is appropriate as RTL styles should override base theme styles.
src/Sylius/Bundle/ShopBundle/Resources/views/layout.html.twig (2)
1-2: RTL macro import is well-structured.Importing the RTL macro at the top of the template follows best practices for template organization.
27-27: RTL attribute application looks good.Applying the RTL attribute conditionally based on the application locale is the correct approach for implementing RTL layout support.
src/Sylius/Bundle/AdminBundle/Resources/views/Layout/layout.html.twig (2)
1-2: RTL macro import follows template best practices.The import statement is appropriately placed at the top of the template file.
24-24: RTL attribute application for admin layout.The implementation correctly applies RTL attributes conditionally based on the application locale, maintaining consistency with the shop layout implementation.
src/Sylius/Bundle/UiBundle/Resources/private/sass/rtl.scss (7)
1-3: Well-structured RTL styles with appropriate selectorUsing the
[dir="rtl"]attribute selector is the correct approach for targeting RTL layout styles, as it works with the HTML attribute set by the Twig macro.
14-20: Admin navigation menu padding correctly adjustedThe padding adjustment for the admin menu headers is appropriate for RTL layout.
22-24: Header icon spacing properly adjustedSetting padding-right for icon content in headers ensures proper spacing in RTL mode.
26-29: Table and dropdown text alignment is appropriateSetting text-align to right for tables and dropdowns is necessary for consistent RTL layout.
31-33: Filter group border fix is appropriateEnsuring border-right-width is set correctly for filter group inputs and selects addresses potential rendering issues in RTL layouts.
35-39: Float and margin adjustments for buttons are correctThe right-floated buttons are properly adjusted to float left in RTL mode, with appropriate margin adjustments.
41-88: Comprehensive RTL adjustments for UI componentsThe adjustments for icons, messages, menus, avatar images, labels, grid alignments, and filters are comprehensive and appropriate for RTL layout. The transformations mirror the layout correctly for right-to-left reading direction.
I particularly appreciate:
- Use of
unsetfor properties that need to be removed- Adjusting background-position for select elements to show dropdown arrows in correct position
- Proper margin adjustments for icons and UI elements
These changes will ensure a consistent RTL experience across the Sylius UI components.
src/Sylius/Bundle/ShopBundle/Resources/private/scss/rtl.scss (6)
1-4: Filter background position correctly adjustedThe select dropdown background position is properly adjusted for RTL layout, ensuring the arrow appears in the correct position.
6-18: Responsive product detail layout properly handledThe media queries for product detail page layouts correctly adjust padding for different screen sizes in RTL context. The use of
!importantis justified here to override any existing styles.
20-26: Grid column shadows correctly handledBox-shadow adjustments for divided grid columns ensure the visual separator appears on the correct side in RTL layouts, with special handling for the first column.
28-36: Step component transformation is impressiveThe step component is properly transformed for RTL layout, with the connector arrow positioning and rotation correctly adjusted. Icon margin is also appropriately set.
38-54: Homepage banner positioning handled wellThe header styling and banner content positioning are correctly adjusted for RTL layout across different screen sizes. The use of
unsetand repositioning is appropriate.
56-71: Horizontal list and address card styling properly adjustedBorder and padding adjustments for horizontal list items and address card buttons are correctly implemented for RTL layout. The separate handling of first/last items ensures consistent spacing.
| Q | A |-----------------|----- | Branch? | 1.14 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | #17767 | License | MIT <!-- - Bug fixes must be submitted against the 1.13 branch - Features and deprecations must be submitted against the 1.14 branch - Features, removing deprecations and BC breaks must be submitted against the 2.0 branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html -->
Summary by CodeRabbit
New Features
Style