fix: schedule imbalance fix #889
Open
+121
−21
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideTo prevent header wrapping when an event has many locales, this PR refactors CSS to enforce flex-nowrap and adds dedicated styling for inline and dropdown locale links (plus user-logs), updates the base template to render locales as an inline list when ≤3 or a dropdown when >3 (including a new script reference), and introduces a small JS module that closes open dropdowns on outside clicks.
Class diagram for dropdown JS moduleclassDiagram
class DropdownManager {
+initDropdowns()
}
class "document" {
}
DropdownManager -- "document" : interacts with
DropdownManager : +initDropdowns()
DropdownManager : +eventListener('click')
DropdownManager : +eventListener('DOMContentLoaded')
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Saksham-Sirohi
requested review from
Sak1012,
hongquan,
mariobehling and
Gagan-Ram
![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:
- Consider extracting the hardcoded
>3threshold for switching between inline locale links and the dropdown into a configurable constant or template variable so it can be adjusted without editing code. - Improve dropdown accessibility by adding keyboard support (e.g. Escape to close, arrow navigation, focus management) and appropriate ARIA attributes on the
<summary>and menu items. - Instead of manually inserting
%3Fwhen building thenextquery parameter, use Django’surlencodeorrequest.get_full_pathto compose the full URL and avoid potential encoding issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the hardcoded `>3` threshold for switching between inline locale links and the dropdown into a configurable constant or template variable so it can be adjusted without editing code.
- Improve dropdown accessibility by adding keyboard support (e.g. Escape to close, arrow navigation, focus management) and appropriate ARIA attributes on the `<summary>` and menu items.
- Instead of manually inserting `%3F` when building the `next` query parameter, use Django’s `urlencode` or `request.get_full_path` to compose the full URL and avoid potential encoding issues.
## Individual Comments
### Comment 1
<location> `app/eventyay/static/cfp/css/_layout.css:49` </location>
<code_context>
align-items: flex-end;
- word-break: break-word;
+ word-break: keep-all;
+ flex-wrap: nowrap;
}
#header-tabs {
</code_context>
<issue_to_address>
**issue:** Using 'flex-wrap: nowrap' may cause layout overflow on smaller screens.
Please verify that the layout remains usable on small screens with many tabs or locale links, as nowrap may prevent content from wrapping and cause overflow.
</issue_to_address>
### Comment 2
<location> `app/eventyay/static/cfp/css/_layout.css:59` </location>
<code_context>
border-radius: var(--size-border-radius) var(--size-border-radius) 0 0;
margin-bottom: -4px;
padding-bottom: 4px;
+ flex-shrink: 0;
a.header-tab {
padding: 8px 10px 4px 10px;
</code_context>
<issue_to_address>
**suggestion:** Adding 'flex-shrink: 0' may reduce flexibility for header elements.
This change may lead to header overflow on smaller screens. Consider applying 'flex-shrink: 0' only when necessary, based on screen size.
Suggested implementation:
```
```
```
#header-tabs {
display: flex;
flex-wrap: nowrap;
z-index: 400;
background: var(--color-bg);
border-radius: var(--size-border-radius) var(--size-border-radius) 0 0;
margin-bottom: -4px;
padding-bottom: 4px;
}
@media (min-width: 600px) {
#header-tabs {
flex-shrink: 0;
}
}
#header-tabs a.header-tab {
padding: 8px 10px 4px 10px;
border-bottom: 2px solid transparent;
color: white;
margin-left: auto;
display: flex;
flex-wrap: nowrap;
flex-shrink: 0;
```
</issue_to_address>
### Comment 3
<location> `app/eventyay/common/templates/common/base.html:109` </location>
<code_context>
+ </summary>
+ <div id="locale-dropdown" class="dropdown-content dropdown-content-s{% if rtl %}e{% else %}w{% endif %}">
+ {% for l, name in request.event.named_locales %}
+ <a href="{% url "cfp:locale.set" event=request.event.slug %}?locale={{ l }}&next={{ request.path }}%3F{{ request.META.QUERY_STRING|urlencode }}"
+ class="dropdown-item {% if l|lower == request.LANGUAGE_CODE|lower %}active{% endif %}" role="menuitem" tabindex="-1">{{ name }}</a>
+ {% endfor %}
</code_context>
<issue_to_address>
**issue:** URL encoding for 'next' parameter may be inconsistent between dropdown and inline links.
Please standardize the URL encoding logic for the 'next' parameter in both dropdown and inline links to prevent navigation errors with complex query strings.
</issue_to_address>
### Comment 4
<location> `app/eventyay/static/common/js/dropdown.js:4-15` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>
### Comment 5
<location> `app/eventyay/static/common/js/dropdown.js:6` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| align-items: flex-end; | ||
| word-break: break-word; | ||
| word-break: keep-all; | ||
| flex-wrap: nowrap; |
There was a problem hiding this comment.
issue: Using 'flex-wrap: nowrap' may cause layout overflow on smaller screens.
Please verify that the layout remains usable on small screens with many tabs or locale links, as nowrap may prevent content from wrapping and cause overflow.
Sak1012
approved these changes
Oct 2, 2025
Gagan-Ram
approved these changes
Oct 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #386 of eventyay-talk high number of locales seemed to be the breaking factor if locales are high a dropdown approach is now used
Summary by Sourcery
Prevent header layout break when there are many locales by introducing a conditional locale selection dropdown, updating CSS layout rules, and adding dropdown behavior via JavaScript.
Bug Fixes:
Enhancements: