-
Notifications
You must be signed in to change notification settings - Fork 0
Multiple UX Polish Updates #72
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,26 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Dropdown button functionality | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function toggleDropdown() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const dropdown = document.getElementById('chartDropdown'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dropdown.classList.toggle('show'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dropdown.classList.toggle('show'); | |
| if (dropdown) { | |
| dropdown.classList.toggle('show'); | |
| } |
Copilot
AI
Dec 14, 2025
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.
Unused variable toggleButton.
| const toggleButton = document.querySelector('.dropdown-toggle'); | |
Copilot
AI
Dec 14, 2025
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.
The click event listener is registered outside of DOMContentLoaded. While it will work since the actual DOM queries happen when the event fires, it's better practice to wrap all event listener registrations in DOMContentLoaded for consistency and to ensure the page structure is ready. This makes the code more maintainable and reduces potential issues if the script load order changes.
| document.addEventListener('click', function(event) { | |
| const dropdown = document.getElementById('chartDropdown'); | |
| const toggleButton = document.querySelector('.dropdown-toggle'); | |
| if (dropdown && !event.target.closest('.dropdown-button-wrapper')) { | |
| dropdown.classList.remove('show'); | |
| } | |
| }); | |
| // Prevent form submission when clicking the dropdown toggle | |
| document.addEventListener('DOMContentLoaded', function() { | |
| // Prevent form submission when clicking the dropdown toggle | |
| document.addEventListener('DOMContentLoaded', function() { | |
| // Close dropdown when clicking outside | |
| document.addEventListener('click', function(event) { | |
| const dropdown = document.getElementById('chartDropdown'); | |
| const toggleButton = document.querySelector('.dropdown-toggle'); | |
| if (dropdown && !event.target.closest('.dropdown-button-wrapper')) { | |
| dropdown.classList.remove('show'); | |
| } | |
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,21 @@ | ||||||||||||
| // Initialize timezone selector with Select2 | ||||||||||||
| document.addEventListener('DOMContentLoaded', function() { | ||||||||||||
| // Initialize Select2 on the timezone select element | ||||||||||||
| $('#timezone_offset').select2({ | ||||||||||||
| placeholder: 'Select or search for a timezone...', | ||||||||||||
| allowClear: false, | ||||||||||||
|
||||||||||||
| allowClear: false, | |
| allowClear: true, |
Copilot
AI
Dec 14, 2025
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.
When restoring the timezone value from localStorage, there's no validation to ensure the stored value is actually one of the valid timezone options. If localStorage contains an invalid or outdated value, Select2 will fail silently. Consider adding validation by checking if the stored value exists as an option in the select element before attempting to restore it.
| if (storedTimezone) { | |
| if ( | |
| storedTimezone && | |
| $('#timezone_offset option[value="' + storedTimezone.replace(/"/g, '\\"') + '"]').length > 0 | |
| ) { |
Copilot
AI
Dec 14, 2025
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.
The timezone-select.js script depends on jQuery being loaded, but there's no check to ensure jQuery is available before using it. If jQuery fails to load from the CDN for any reason, this script will throw an error and break the page. Add a check like if (typeof jQuery === 'undefined') or wrap the code in a try-catch block for better error handling.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,8 +35,12 @@ | |||||||||
| <link rel="preconnect" href="https://fonts.googleapis.com"> | ||||||||||
| <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin> | ||||||||||
| <link href="https://fonts.googleapis.com/css2?family=Space+Mono:ital,wght@0,400;0,700;1,400;1,700&display=swap" rel="stylesheet"> | ||||||||||
| <!-- Select2 for searchable dropdowns --> | ||||||||||
| <link href="https://cdn.jsdelivr.net/npm/select2@4.1.0-rc.0/dist/css/select2.min.css" rel="stylesheet" /> | ||||||||||
| <link rel="stylesheet" href="{{ url_for('static', filename='css/style.css') }}"> | ||||||||||
| <!-- Dark mode script loaded early to prevent flash --> | ||||||||||
| <script src="https://code.jquery.com/jquery-3.6.0.min.js"></script> | ||||||||||
| <script src="https://cdn.jsdelivr.net/npm/select2@4.1.0-rc.0/dist/js/select2.min.js"></script> | ||||||||||
|
Comment on lines
+42
to
+43
|
||||||||||
| <script src="https://code.jquery.com/jquery-3.6.0.min.js"></script> | |
| <script src="https://cdn.jsdelivr.net/npm/select2@4.1.0-rc.0/dist/js/select2.min.js"></script> | |
| <script src="https://code.jquery.com/jquery-3.6.0.min.js" defer></script> | |
| <script src="https://cdn.jsdelivr.net/npm/select2@4.1.0-rc.0/dist/js/select2.min.js" defer></script> |
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.
The
!importantflag is used here to override Select2's default width. While this works, it's generally better to avoid!importantas it makes styles harder to maintain and override in the future. Consider using a more specific selector or setting the width through Select2's initialization options in timezone-select.js instead.