Conversation
WalkthroughThis update introduces UI enhancements and feature placeholders related to role management in the admin panel. It adds a new "Administration" submenu with a Sylius Plus promotional link, a roles selection UI fragment, supporting translations, and visual effects. Several Behat test navigation methods are refactored for improved reliability. Minor configuration and workflow formatting adjustments are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin User
participant Menu as MainMenuBuilder
participant UI as Admin UI
participant Plus as Sylius Plus Site
Admin->>Menu: Accesses Admin Panel
Menu->>UI: Renders sidebar with "Administration" submenu
UI->>Admin: Displays "roles" link with Plus badge
Admin->>UI: Clicks "roles" link or roles segment
UI->>Plus: Opens Sylius Plus roles URL in new tab
Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
.github/workflows/ci_e2e-mysql.yaml (1)
186-189: Improved readability with multi-line formatting.Breaking the long Behat command into multiple lines with continuation characters improves readability while maintaining the same functionality.
There's a trailing space at the end of line 186 that should be removed:
-run: +run:🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 186-186: trailing spaces
(trailing-spaces)
config/packages/security.yaml (1)
17-17: Fix YAMLlint warning about extra spaces.There are too many spaces after the colon in this line.
- pattern: ^/(_(profiler|wdt)|css|images|js)/ + pattern: ^/(_(profiler|wdt)|css|images|js)/🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 17-17: too many spaces after colon
(colons)
src/Sylius/Bundle/UiBundle/Resources/translations/messages.en.yml (1)
740-743: Added role name translations.The translation keys for the predefined admin roles are correctly added.
Note that there's a minor spelling inconsistency: "Fulfilment" in the translation uses British English spelling (line 742), while the key uses American English spelling "fulfillment". Consider standardizing the spelling for consistency.
src/Sylius/Bundle/AdminBundle/Resources/private/sass/_ui.scss (2)
105-112: Flexbox alignment for menu items with Plus badgesThe CSS flexbox layout ensures proper alignment of the Plus badge within menu items.
Note: The use of
!importantflag is generally discouraged, but might be necessary here if there are competing styles. Consider removing it if possible to maintain cleaner specificity hierarchy.
382-424: Blur effect implementation has redundant rulesThe Plus feature UI enhancement with blur effect and overlay image works well, but contains redundant CSS rules.
The blur effect and pointer-events properties are defined twice:
- Within
.blur-content:hover(lines 389-395)- Within
.plus-blur:hover .blur-content(lines 416-422)This duplication might cause maintenance issues if one set is updated and the other forgotten.
.plus-blur { position: relative; overflow: hidden; .blur-content { transition: filter 0.3s ease; - - &:hover { - filter: blur(2px); - - * { - pointer-events: none; - } - } } .overlay-image { position: absolute; top: 50%; left: 50%; transform: translate(-50%, -50%); opacity: 0; z-index: 10; transition: opacity 0.3s ease; pointer-events: none; max-width: 80%; height: auto; } &:hover { .overlay-image { opacity: 1; } .blur-content { filter: blur(2px); * { pointer-events: none; } } } }src/Sylius/Bundle/AdminBundle/Resources/views/AdminUser/Form/_roles.html.twig (3)
1-2: Consider extracting URL to configurationThe promotional URL with UTM parameters is hardcoded in both this template and in
MainMenuBuilder.php. Consider extracting this to a configuration parameter to maintain consistency and enable easier updates.
7-15: Roles label should use translation keyWhile most labels in this template use translation keys, the label "Roles" is hardcoded in English at line 8. For consistency with the rest of the application and to support localization, this should use a translation key.
<div class="field"> - <label for="sylius_admin_user_rolesResources">Roles</label> + <label for="sylius_admin_user_rolesResources">{{ 'sylius.ui.roles'|trans }}</label> <select multiple id="sylius_admin_user_rolesResources"> <option>{{ 'sylius.ui.role_super_admin'|trans }}</option> <option>{{ 'sylius.ui.role_product_manager'|trans }}</option> <option>{{ 'sylius.ui.role_fulfillment_worker'|trans }}</option> <option>{{ 'sylius.ui.role_channel_admin'|trans }}</option> </select> </div>
17-24: Non-functional UI elements could use form-disabled attributeSince these UI elements are non-functional placeholders (they don't submit data), consider adding a
disabledattribute to clearly indicate their non-functional status, especially for assistive technologies.<div class="field"> <div class="ui toggle checkbox"> - <input type="checkbox" id="sylius_admin_user_enablePermissionChecker" value="1" checked class="hidden"> + <input type="checkbox" id="sylius_admin_user_enablePermissionChecker" value="1" checked class="hidden" disabled> <label for="sylius_admin_user_enablePermissionChecker"> {{ 'sylius.ui.enabled_permission_checker'|trans }} </label> </div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/Sylius/Bundle/AdminBundle/Resources/private/img/sylius-plus-sidebar.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
.github/workflows/ci_e2e-mysql.yaml(1 hunks)config/packages/security.yaml(1 hunks)src/Sylius/Behat/Element/Product/ShowPage/MediaElement.php(1 hunks)src/Sylius/Behat/Page/Admin/Product/UpdateConfigurableProductPage.php(1 hunks)src/Sylius/Behat/Page/Admin/Product/UpdateSimpleProductPage.php(1 hunks)src/Sylius/Behat/Page/Admin/Taxon/UpdatePage.php(1 hunks)src/Sylius/Behat/Page/Shop/Product/ShowPage.php(1 hunks)src/Sylius/Bundle/AdminBundle/Menu/MainMenuBuilder.php(2 hunks)src/Sylius/Bundle/AdminBundle/Resources/config/app/events.yaml(1 hunks)src/Sylius/Bundle/AdminBundle/Resources/private/entry.js(1 hunks)src/Sylius/Bundle/AdminBundle/Resources/private/sass/_ui.scss(2 hunks)src/Sylius/Bundle/AdminBundle/Resources/views/AdminUser/Form/_roles.html.twig(1 hunks)src/Sylius/Bundle/UiBundle/Resources/translations/messages.en.yml(2 hunks)src/Sylius/Bundle/UiBundle/Resources/views/Menu/sidebar.html.twig(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/Sylius/Behat/Element/Product/ShowPage/MediaElement.php (1)
src/Sylius/Behat/Page/Shop/Product/ShowPage.php (2)
getCurrentUrl(162-165)visit(307-311)
src/Sylius/Behat/Page/Admin/Product/UpdateSimpleProductPage.php (1)
src/Sylius/Behat/Page/Shop/Product/ShowPage.php (2)
getCurrentUrl(162-165)visit(307-311)
src/Sylius/Behat/Page/Admin/Product/UpdateConfigurableProductPage.php (1)
src/Sylius/Behat/Page/Shop/Product/ShowPage.php (2)
getCurrentUrl(162-165)visit(307-311)
src/Sylius/Behat/Page/Admin/Taxon/UpdatePage.php (1)
src/Sylius/Behat/Page/Shop/Product/ShowPage.php (2)
getCurrentUrl(162-165)visit(307-311)
🪛 YAMLlint (1.35.1)
config/packages/security.yaml
[warning] 17-17: too many spaces after colon
(colons)
.github/workflows/ci_e2e-mysql.yaml
[error] 186-186: trailing spaces
(trailing-spaces)
🔇 Additional comments (12)
src/Sylius/Behat/Element/Product/ShowPage/MediaElement.php (1)
27-31: Navigation logic improved with explicit URL handling.This change enhances test reliability by storing the original URL and explicitly navigating back to it instead of using browser history navigation with
back(). This approach is more robust against browser history inconsistencies.src/Sylius/Behat/Page/Admin/Product/UpdateConfigurableProductPage.php (1)
98-102: Navigation logic improved with explicit URL handling.Similar to the changes in other Behat page classes, this change replaces browser history navigation (
back()) with explicit URL restoration. This ensures the test returns to exactly the right page after image validation.src/Sylius/Behat/Page/Admin/Product/UpdateSimpleProductPage.php (1)
225-229: Navigation logic improved with explicit URL handling.The change follows the pattern implemented in other Behat page classes, replacing browser history navigation with explicit URL revisiting. This approach provides more consistent behavior across different browser environments during testing.
src/Sylius/Behat/Page/Admin/Taxon/UpdatePage.php (1)
84-88: Improved reliability in page navigation.The change replaces the browser's back navigation mechanism with a more reliable approach that explicitly stores and revisits the original URL. This pattern makes the test more robust, especially in headless browser environments where the back navigation can be unreliable.
src/Sylius/Behat/Page/Shop/Product/ShowPage.php (1)
270-275: Enhanced test reliability with explicit URL navigation.This change follows the same reliable pattern implemented in the admin pages - storing the original URL and explicitly revisiting it instead of relying on the browser's history navigation. This approach is more dependable in automated testing environments and ensures consistent test behavior.
config/packages/security.yaml (1)
16-19: Correctly reordered dev firewall to the top.The
devfirewall is appropriately moved to the top of the firewall configuration. This is a good security practice since Symfony processes firewalls in order and uses the first match. Placing the development-specific pattern first ensures these paths are properly handled before other more general patterns.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 17-17: too many spaces after colon
(colons)
src/Sylius/Bundle/AdminBundle/Resources/private/entry.js (1)
13-13: Successfully imported Sylius Plus sidebar asset.The SVG asset for the Sylius Plus sidebar badge is properly imported. This asset will be included in the build and available for use in the templates.
src/Sylius/Bundle/AdminBundle/Resources/config/app/events.yaml (1)
1068-1070: Added roles form block with appropriate priority.The new roles block is correctly positioned with a priority of 15, placing it between the existing "additional_information" (priority 20) and "preferences" (priority 10) blocks in the admin user form second column.
src/Sylius/Bundle/UiBundle/Resources/translations/messages.en.yml (1)
334-334: Added permission checker translation.The translation key for the permission checker toggle is properly added.
src/Sylius/Bundle/UiBundle/Resources/views/Menu/sidebar.html.twig (1)
45-51: Enhanced menu item rendering with Sylius Plus badge.The changes add proper support for displaying a Sylius Plus badge for menu items with the
plus_logoextra attribute. Theitem-contentclass addition will help with styling the layout to accommodate the badge.src/Sylius/Bundle/AdminBundle/Menu/MainMenuBuilder.php (2)
39-39: LGTM - Administration submenu integrationThe new submenu reference is properly added to the menu creation sequence.
373-388: Implementation follows established menu patternsThe implementation of the Administration submenu follows the same patterns as other submenus in this class. The method properly creates a submenu item with appropriate icons and label, and adds a child item with Sylius Plus UTM tracking parameters.
A few points to note:
- The icon uses
tabler:lockformat which is slightly different from other icons in the file- The UTM parameters will help track engagement with this promotional feature
- The
plus_logoextra attribute is properly set to enable the badge rendering
Updated plus utm link
Summary by CodeRabbit
New Features
Style
Bug Fixes
Documentation