-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Admin] Define title icon and subheader on Twig hooks configuration #17592
Conversation
WalkthroughThe pull request modifies a Twig template for header rendering in the Sylius Admin Bundle. Two new variables, Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Sylius/Bundle/AdminBundle/templates/shared/crud/common/content/header/title_block/title.html.twig (1)
10-11
: Consider providing meaningful fallback values for 'icon' and 'subheader'.
Using null as a default is valid, but if there's a scenario where an icon or subheader is typically expected, you might display fallback text or a placeholder icon instead, improving user experience and clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Sylius/Bundle/AdminBundle/templates/shared/crud/common/content/header/title_block/title.html.twig
(1 hunks)
🔇 Additional comments (1)
src/Sylius/Bundle/AdminBundle/templates/shared/crud/common/content/header/title_block/title.html.twig (1)
16-16
: Verify argument order in 'header.h1'.
Ensure the underlying macro or method supports the newly added parameters in the exact sequence (header, icon, subheader). If the order is different, unexpected results could occur.
✅ Verification successful
Let me verify the argument order in the header.h1
macro by first locating and examining its definition.
The argument order in header.h1
is correct and matches the usage
The macro definition found in src/Sylius/Bundle/AdminBundle/templates/shared/helper/header.html.twig
shows that header.h1
accepts exactly three parameters in the order: header
, icon
, subheader
, which perfectly matches how it's being used in the code under review.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for header.h1 macro definition in twig files
fd -e twig -x rg -l "macro h1" {}
# Also get the content of any files that might contain this macro
fd -e twig -x rg -A 5 "macro h1" {}
Length of output: 257
Bunnyshell Preview Environment deletedAvailable commands:
|
Thank you @loic425! 🎉 |
Improvement to #17579
And also related to Sylius/Stack#193
Summary by CodeRabbit