-
Notifications
You must be signed in to change notification settings - Fork 10
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
Adds period filter to event page #672
Conversation
ronnyjohnti
commented
Mar 27, 2025
Q | A |
---|---|
Branch? | feat/event-date-filter |
Bug fix? | no |
New feature? | yes |
Deprecations? | no |
Issues | Fix #453 |
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.
Pull Request Overview
This PR adds a period filter feature for the event page along with its associated UI elements, translations, tests, and developer tooling configurations.
- Adds new translation keys in both Spanish and English for period filtering and calendar actions.
- Updates Cypress tests to verify the correct functionality of the period filter.
- Introduces new JavaScript code to handle the period filter with an AirDatepicker integration and adjusts Docker configuration for Xdebug.
Reviewed Changes
Copilot reviewed 14 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
translations/messages.es.yaml | Adds new translation keys for period filtering and calendar labels. |
translations/messages.en.yaml | Adds new translation keys for period filtering and calendar labels. |
docker-compose.yml | Adds volume mapping for an Xdebug configuration file. |
cypress/e2e/web/event/event-list.cy.js | Updates test scenario to cover period filter selections. |
config/routes/web_profiler.yaml | Configures profiler routes for development. |
config/packages/web_profiler.yaml | Adds profiler and toolbar configurations for dev and test environments. |
assets/js/side-filter.js | Implements calendar integration and period filter behavior. |
assets/js/navbar-dropdown.js | Refactors dropdown functionality with optional chaining. |
assets/app.js | Imports CSS for the AirDatepicker library. |
Files not reviewed (11)
- assets/styles/lib/air-datepicker-custom.css: Language not supported
- composer.json: Language not supported
- config/bundles.php: Language not supported
- docker/php/99-xdebug.ini: Language not supported
- importmap.php: Language not supported
- package.json: Language not supported
- src/Controller/Web/EventWebController.php: Language not supported
- src/Repository/EventRepository.php: Language not supported
- src/Repository/Interface/EventRepositoryInterface.php: Language not supported
- src/Service/EventService.php: Language not supported
- templates/event/_partials/side-filter.html.twig: Language not supported
Comments suppressed due to low confidence (2)
assets/js/side-filter.js:76
- [nitpick] Consider using a boolean value (true) for the 'selected' property instead of the string 'selected' to better reflect its intended use.
customPeriod.selected = 'selected';
assets/js/side-filter.js:7
- The variable name 'BTN_CLOSE_FILER' appears to be a typographical error. Consider renaming it to 'BTN_CLOSE_FILTER' for clarity.
const BTN_CLOSE_FILER = document.getElementById('close-filter');
f29c684
to
f5b9694
Compare
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.
Pull Request Overview
This PR introduces a period filter to the event page by adding new translations for the filter labels, updating the tests to cover the new UI interactions, and implementing the period filter functionality using AirDatepicker.
- Updated Spanish and English translations with new keys for the period filter.
- Modified Cypress tests to simulate and validate the period filter selection.
- Fixed a naming typo in the side filter code and added a new period-filter module.
Reviewed Changes
Copilot reviewed 12 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
translations/messages.es.yaml | Added new Spanish translations for period filter related keys. |
translations/messages.en.yaml | Added new English translations and rearranged related keys. |
docker-compose.yml | Added Xdebug configuration via an extra volume mount. |
cypress/e2e/web/event/event-list.cy.js | Updated tests to cover new period filter UI interactions on the event page. |
assets/js/side-filter.js | Fixed a naming typo with BTN_CLOSE_FILTER and ensured consistency in style assignment. |
assets/js/navbar-dropdown.js | Added optional chaining for robustness in dropdown functionality. |
assets/js/event/period-filter.js | Introduced new period filter functionality using AirDatepicker. |
assets/app.js | Imported new CSS for the AirDatepicker. |
Files not reviewed (12)
- Makefile: Language not supported
- assets/styles/lib/air-datepicker-custom.css: Language not supported
- composer.json: Language not supported
- docker/php/99-xdebug.ini: Language not supported
- importmap.php: Language not supported
- package.json: Language not supported
- src/Controller/Web/EventWebController.php: Language not supported
- src/Repository/EventRepository.php: Language not supported
- src/Repository/Interface/EventRepositoryInterface.php: Language not supported
- src/Service/EventService.php: Language not supported
- templates/event/_partials/side-filter.html.twig: Language not supported
- templates/event/list.html.twig: Language not supported
f5b9694
to
671bb33
Compare
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.
Pull Request Overview
This PR adds a new period filter feature to the event page, allowing users to select a date range using an interactive calendar. Key changes include updated translation files for new filter labels, additional Cypress end-to-end tests to validate the filtering behavior, and a new JavaScript module to integrate AirDatepicker for period selection.
Reviewed Changes
Copilot reviewed 12 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
translations/messages.es.yaml | Added Spanish translations for new filter labels |
translations/messages.en.yaml | Added English translations for new filter labels and corrected ordering |
docker-compose.yml | Added an Xdebug configuration volume |
cypress/e2e/web/event/event-list.cy.js | Updated test selectors and assertions to validate the period filter |
assets/js/side-filter.js | Fixed a typo in variable naming and standardized opacity values |
assets/js/navbar-dropdown.js | Enhanced null-safety by using optional chaining in dropdown handling |
assets/js/event/period-filter.js | New module integrating AirDatepicker for period filter functionality |
assets/app.js | Imported required stylesheets for AirDatepicker |
Files not reviewed (12)
- Makefile: Language not supported
- assets/styles/lib/air-datepicker-custom.css: Language not supported
- composer.json: Language not supported
- docker/php/99-xdebug.ini: Language not supported
- importmap.php: Language not supported
- package.json: Language not supported
- src/Controller/Web/EventWebController.php: Language not supported
- src/Repository/EventRepository.php: Language not supported
- src/Repository/Interface/EventRepositoryInterface.php: Language not supported
- src/Service/EventService.php: Language not supported
- templates/event/_partials/side-filter.html.twig: Language not supported
- templates/event/list.html.twig: Language not supported
Comments suppressed due to low confidence (2)
assets/js/event/period-filter.js:13
- The use of 'multipleDates: true' is redundant when 'range: true' is already specified. Consider removing it if it is not required.
multipleDates: true,
translations/messages.en.yaml:255
- The translation for 'name_initiative' appears incorrect. Consider replacing 'Nome gives initiative' with a phrase like 'Initiative Name'.
name_initiative: Nome gives initiative