-
Notifications
You must be signed in to change notification settings - Fork 103
Add menu items in tickets page #814
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
base: development
Are you sure you want to change the base?
Add menu items in tickets page #814
Conversation
Reviewer's GuideRefactor ticket page navigation by extracting the inlined menu into a reusable template fragment and including it on both the event index and order pages to eliminate duplication. Class diagram for template structure after navigation menu refactorclassDiagram
class index.html {
+includes fragment_nav.html
}
class order.html {
+includes fragment_nav.html
}
class fragment_nav.html {
+navigation menu markup
}
index.html --> fragment_nav.html : includes
order.html --> fragment_nav.html : includes
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @Gagan-Ram - I've reviewed your changes - here's some feedback:
- Ensure that you pass all required context variables (e.g. with_margin, is_video_plugin_enabled) into fragment_nav.html when including it so the navigation layout remains consistent.
- The “Tickets” button in the new fragment lacks an href attribute—please confirm this is intentional or add the proper URL to maintain its functionality.
- You’ve switched from using {% trans %} to {% translate %} in the fragment—consider standardizing on one for consistency across your templates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Ensure that you pass all required context variables (e.g. with_margin, is_video_plugin_enabled) into fragment_nav.html when including it so the navigation layout remains consistent.
- The “Tickets” button in the new fragment lacks an href attribute—please confirm this is intentional or add the proper URL to maintain its functionality.
- You’ve switched from using {% trans %} to {% translate %} in the fragment—consider standardizing on one for consistency across your templates.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Please add a screenshot. Thanks |
If you're able to see the menu items on the Tickets Shop page, then they should also be visible on the Order Tickets page. |
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.
@Gagan-Ram please add a video showcasing the changes
Fixes: #807
Summary by Sourcery
Extract the event navigation menu into a reusable template fragment and include it in both the tickets index and order pages, ensuring consistent menu items across both views.
New Features:
Enhancements: