Align Tickets and Talk menu design with Common Dashboard Menu #1547
Conversation
Reviewer's GuideUpdates the event dashboard header structure and shared navigation styles so that Tickets and Talk menus visually align with the Common Dashboard menu in spacing, typography, and button placement. Flow diagram for updated event dashboard header layoutflowchart TD
main_title["div id=main-title"]
navigation_title["div.navigation-title"]
heading["h1"]
event_name["text: event.name"]
event_date["span.event-date text-muted"]
navigation_button["div.navigation-button (component_link.html)"]
main_title --> navigation_title
navigation_title --> heading
heading --> event_name
heading --> event_date
main_title --> navigation_button
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The new
#main-titlestyling in_layout.cssoverlaps conceptually with the.page-heading .navigationstyles incustom.scss; consider reusing a shared class or extending the existing pattern instead of introducing a separate ID-based layout to avoid divergence over time. - Applying
text-align: centerandfont-weight: 200to.page-heading .navigationincustom.scsswill affect all pages using this structure; double-check that this global change is intended for every dashboard and not just Tickets/Talks, or scope it more narrowly if needed. - Switching the event title from an
h2to anh1changes the heading hierarchy on the dashboard; verify that this still fits the overall page structure or adjust other headings accordingly to maintain accessible, sequential heading levels.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `#main-title` styling in `_layout.css` overlaps conceptually with the `.page-heading .navigation` styles in `custom.scss`; consider reusing a shared class or extending the existing pattern instead of introducing a separate ID-based layout to avoid divergence over time.
- Applying `text-align: center` and `font-weight: 200` to `.page-heading .navigation` in `custom.scss` will affect all pages using this structure; double-check that this global change is intended for every dashboard and not just Tickets/Talks, or scope it more narrowly if needed.
- Switching the event title from an `h2` to an `h1` changes the heading hierarchy on the dashboard; verify that this still fits the overall page structure or adjust other headings accordingly to maintain accessible, sequential heading levels.
## Individual Comments
### Comment 1
<location> `app/eventyay/orga/templates/orga/event/dashboard.html:28-37` </location>
<code_context>
</div>
{% endif %}
- <div id="main-title" class="d-md-flex justify-content-between">
- <h2>
- <span>
+ <div id="main-title">
+ <div class="navigation-title">
+ <h1>
{{ request.event.name }}
- <small class="text-muted small">{{ request.event.get_date_range_display }}</small>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Switching from h2 to h1 might disrupt the overall heading hierarchy of the page/layout.
If the layout or parent templates already define a primary `<h1>` (e.g. at site or section level), this change may introduce multiple top-level headings on a single page, harming accessibility and document structure. Please verify the global heading hierarchy and consider keeping this as a lower-level heading, using CSS for visual emphasis if needed.
```suggestion
<div id="main-title">
<div class="navigation-title">
<h2>
{{ request.event.name }}
<span class="event-date text-muted">
{{ request.event.get_date_range_display }}
</span>
</h2>
</div>
{% include "orga/event/component_link.html" %}
</div>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR aligns the event dashboard header layout in the Tickets and Talk menus with the Common Dashboard navigation for consistent typography, spacing, and behavior. The main focus is on unifying the visual presentation of event dashboards across different sections of the application.
Key Changes
- Introduces a reusable
#main-titlelayout structure with navigation title and action button areas in the event dashboard - Updates HTML structure from
<h2>to<h1>with structured date display for better semantic hierarchy - Adds consistent styling properties (text-align, font-weight, margin-bottom) to common dashboard navigation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
app/eventyay/static/orga/css/_layout.css |
Adds new #main-title CSS rules for flex layout, typography, and event date styling |
app/eventyay/static/eventyay-common/scss/custom.scss |
Enhances .navigation class with centered text alignment, lighter font weight, and bottom margin |
app/eventyay/orga/templates/orga/event/dashboard.html |
Restructures header from <h2> to <h1> within .navigation-title wrapper, updates event date markup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #main-title { | ||
| display: flex; | ||
| flex-wrap: wrap; | ||
| text-align: center; | ||
| font-weight: 200; | ||
| } | ||
|
|
||
| #main-title .navigation-button { | ||
| margin-left: auto; | ||
| } | ||
|
|
||
| #main-title .navigation-title h1 { | ||
| font-size: 2.25rem; | ||
| font-weight: 500; | ||
| line-height: 1.2; | ||
| } | ||
|
|
||
| #main-title .navigation-title h1 .event-date { | ||
| font-size: 65%; | ||
| font-weight: 400; | ||
| line-height: 1; | ||
| } |
There was a problem hiding this comment.
There's significant duplication between these styles and the .navigation styles in custom.scss. Both define display: flex, flex-wrap: wrap, text-align: center, and font-weight: 200. Additionally, both have .navigation-button { margin-left: auto; }. Consider consolidating these styles into a single shared location to maintain DRY principles and prevent style drift between the two implementations.
| .header-nav.btn-success.active { | ||
| box-shadow: inset 0 3px 5px rgba(0, 0, 0, 0.125); | ||
| } | ||
|
|
There was a problem hiding this comment.
Consider adding a comment explaining the purpose of this style block and its relationship to the common dashboard navigation styles. This would help future maintainers understand why these specific styles are needed for the event dashboard.
| /* Event dashboard main title area | |
| * These styles control the layout of the main header/title section | |
| * for the event dashboard. The flex container aligns the page title | |
| * with the navigation button, and works together with the common | |
| * header navigation button styles defined above to create a cohesive | |
| * dashboard header. | |
| */ |
Saksham-Sirohi
left a comment
Saksham-Sirohi
left a comment
There was a problem hiding this comment.
Hi ui is a bit different on Talk platform admin button also appears differently and fonts appear different then tickets
Saksham-Sirohi
left a comment
Saksham-Sirohi
left a comment
There was a problem hiding this comment.
Hi ui is a bit different on Talk platform admin button also appears differently and fonts appear different then tickets
prayag78
force-pushed
the
align-menu-with-dashboard
branch
from
4c8b8eb to
9848fce
Compare
|
Please update screenshots once ready. |
|
Hi @Saksham-Sirohi , sorry for the late update. Screen.Recording.2025-12-20.113012.mp4 |
Hi @mariobehling , I’ve updated the screenshots |
|
Fixed the bottom spacing. On large screens, Talk doesn’t have a sidebar I think it causes spacing shift |
|
Please provide an updated video. |
Recording.2025-12-20.144825.mp4 |
|
It looks like the dropdown user menu has disappeared in the talk component. |
|
Please also provide the view on large screens either on video or screenshots. |
It exists, but on my setup maybe it didn’t render properly |
Recording.2025-12-20.152605.mp4 |
Please ensure that it works. We need to be sure that this issue is not related to your PR. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #page-content { | ||
| background: var(--color-bg); | ||
| flex-grow: 1; | ||
| max-width: 100%; |
There was a problem hiding this comment.
The change from padding: 20px 40px to padding: 18px 44px introduces asymmetric adjustments (-2px vertical, +4px horizontal). This creates a non-uniform scaling pattern that may be difficult to maintain. Consider using a consistent scaling factor or documenting the specific design rationale for these particular values to aid future maintenance.
| max-width: 100%; | |
| max-width: 100%; | |
| /* Note: Asymmetric padding (18px vertical, 44px horizontal) is intentional to match surrounding layout spacing. */ |
| border-radius: 0 !important; | ||
| font-size: 16px; | ||
| font-weight: normal; | ||
| padding: 6px 10px; |
There was a problem hiding this comment.
Inconsistent padding values between similar button styles. In _layout.css, the padding is set to "6px 10px", while in custom.scss the padding is "7px 7px". These should be aligned for consistency across the dashboard menus.
| padding: 6px 10px; | |
| padding: 7px 7px; |
Yes, I’m trying to find why it didn’t render in the UI on my setup, but this PR doesn’t touch any files related to the dropdown icon. |
mariobehling
left a comment
mariobehling
left a comment
There was a problem hiding this comment.
Thanks! Please also check latest AI comments.
4 participants
Summary
Aligns Tickets and Talk menus with the Common Dashboard for consistent spacing and behavior.
Fixes #1542
Before
After
Summary by Sourcery
Align event dashboard header layout with the common dashboard navigation for consistent typography and spacing.
New Features:
Enhancements: