[Admin] Improve product grid layout and main taxon display#17618
[Admin] Improve product grid layout and main taxon display#17618GSadee merged 1 commit intoSylius:2.0from
Conversation
WalkthroughThe pull request introduces modifications to the Sylius Admin Bundle's product grid configuration and main taxon template. The changes focus on adjusting column widths in the product grid and improving the display of the main taxon's name, including a new tooltip mechanism for long category names. These updates reflect a reallocation of column widths and a refined error message style, contributing to the overall visual presentation in the admin product listing. Changes
Sequence DiagramsequenceDiagram
participant UI as Admin Grid
participant Data as Product Data
participant Template as Main Taxon Template
UI->>Data: Retrieve Product Details
Data-->>Template: Send Product Information
Template->>Template: Split Category Name
alt Long Category Name
Template->>UI: Render Truncated Name with Tooltip
else Short/Normal Name
Template->>UI: Render Full Name
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
9f62ac2 to
59e19bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Sylius/Bundle/AdminBundle/templates/product/grid/field/main_taxon.html.twig (1)
9-9: Consider using a more distinct style for the error state.While changing from danger to muted style makes the UI less aggressive, it might make it harder for users to identify problematic entries. Consider using a middle-ground approach.
- <span class="text-muted">{{ ux_icon('tabler:exclamation-circle') }} {{ 'sylius.ui.no_main_taxon'|trans }}</span> + <span class="text-warning">{{ ux_icon('tabler:exclamation-circle') }} {{ 'sylius.ui.no_main_taxon'|trans }}</span>src/Sylius/Bundle/AdminBundle/Resources/config/grids/product.yml (1)
20-21: Consider adding responsive classes for the image column.The
w-1class might make the image too small on larger screens. Consider using Bootstrap's responsive width classes.vars: - th_class: "w-1" + th_class: "w-1 w-sm-2 w-md-3"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Sylius/Bundle/AdminBundle/Resources/config/grids/product.yml(1 hunks)src/Sylius/Bundle/AdminBundle/templates/product/grid/field/main_taxon.html.twig(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Static checks / PHP 8.3, Symfony ^7.1
- GitHub Check: Static checks / PHP 8.2, Symfony ^6.4
🔇 Additional comments (2)
src/Sylius/Bundle/AdminBundle/templates/product/grid/field/main_taxon.html.twig (1)
2-7: LGTM! Clean implementation of category path truncation with tooltip.The solution elegantly handles long category paths by:
- Showing only the last segment
- Indicating truncation with ".. /"
- Providing full path on hover using Bootstrap's tooltip
src/Sylius/Bundle/AdminBundle/Resources/config/grids/product.yml (1)
20-21: Verify grid layout with various content lengths.The column width adjustments (image: w-1, name: w-33) combined with the removal of code column width might need validation with:
- Products with very long names
- Products with long category paths (even with the new tooltip)
- Various screen sizes
Let's verify the impact:
Also applies to: 29-29
✅ Verification successful
Width classes follow established grid patterns
The column width adjustments (w-1 for image, w-33 for name) align with Sylius's standard grid layout patterns, as evidenced by similar usage across multiple entity grids. These classes are part of a well-tested responsive system used throughout the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for product grid templates and related CSS to understand layout constraints rg -t html -t css "sylius_admin_product|product-grid" --glob '!node_modules' # Look for any existing responsive layout tests rg -t php "responsive|mobile|screen" --glob '*Test.php' --glob '!vendor'Length of output: 369
Script:
#!/bin/bash # Search for grid templates and layout patterns rg -t html "grid|table" src/Sylius/Bundle/AdminBundle/Resources/views rg -t yaml "grid:" src/Sylius/Bundle/AdminBundle/Resources/config # Look for CSS framework configuration or utility classes fd -e css . src/Sylius/Bundle/AdminBundle/Resources fd -e scss . src/Sylius/Bundle/AdminBundle/Resources # Search for UI/Behat testing infrastructure fd -e feature . tests/Behat rg -t php "WebTestCase|KernelBrowser" testsLength of output: 10960
Script:
#!/bin/bash # Check datatable styles implementation cat src/Sylius/Bundle/AdminBundle/Resources/assets/styles/_datatable.scss # Look for grid templates in UiBundle as it might contain base templates fd -e html.twig . src/Sylius/Bundle/UiBundle/Resources/views # Search for width class usage patterns rg "w-[0-9]+" --type-add 'template:*.{twig,scss,css,yml,yaml}' -t templateLength of output: 12873
Bunnyshell Preview Environment deletedAvailable commands:
|
| @@ -1,7 +1,10 @@ | |||
| {% if data.fullname is defined %} | |||
| {{ data.fullname }} | |||
| {% set categories = data.fullname|split('/') %} | |||
There was a problem hiding this comment.
If taxon name contains / character, then split won't work as expected.
59e19bb to
813b78f
Compare
Change in category path display. The proposed change is to display the full path only on hover, as the path can be very long and disrupt the table.
Before
After
Summary by CodeRabbit