-
Notifications
You must be signed in to change notification settings - Fork 242
[QA] Set control multi select tests #1558
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: main
Are you sure you want to change the base?
Conversation
This reverts commit 8795181.
…control_multi_select_tests
View the example dashboards of the current commit live on PyCafe ☕ 🚀Updated on: 2026-01-27 15:34:08 UTC Compare the examples using the commit's wheel file vs the latest released version: vizro-core/examples/scratch_devView with commit's wheel vs View with latest release vizro-core/examples/dev/View with commit's wheel vs View with latest release vizro-core/examples/visual-vocabulary/View with commit's wheel vs View with latest release vizro-core/examples/tutorial/ |
…control_multi_select_tests
…control_multi_select_tests
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 comprehensive end-to-end tests for the set control multi-select functionality in Vizro. The tests cover various interaction patterns including graph clicks with modifier keys, AgGrid row selection with checkboxes and command/shift keys, button/card triggers, and filtered component interactions.
Changes:
- Added new test helper functions for improved path generation and modifier key interactions
- Created comprehensive test suite for multi-select set control actions covering graphs, AgGrid tables, buttons, and cards
- Added new test pages demonstrating multi-select scenarios including cross-filtering, self-filtering, and filtered triggers
- Refactored existing tests to use new helper functions for better maintainability
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| vizro-core/tests/tests_utils/e2e/vizro/paths.py | Added helper functions for AgGrid cell/checkbox paths and scatter point paths (contains critical syntax error) |
| vizro-core/tests/tests_utils/e2e/vizro/navigation.py | Added modifier_click function for keyboard+mouse interactions |
| vizro-core/tests/tests_utils/e2e/vizro/constants.py | Added constants for new test pages and components |
| vizro-core/tests/e2e/vizro/test_screenshots/test_screenshots.py | Refactored to use new table_ag_grid_cell_path_by_row helper |
| vizro-core/tests/e2e/vizro/test_dom_elements/test_table_ag_grid.py | Refactored to use new path helper functions |
| vizro-core/tests/e2e/vizro/test_dom_elements/test_dynamic_data.py | Removed redundant select_all_status=False parameters |
| vizro-core/tests/e2e/vizro/test_dom_elements/test_actions.py | Added 10 new comprehensive tests for multi-select set control functionality |
| vizro-core/tests/e2e/vizro/dashboards/default/pages/set_control_multi_select_pages.py | Created new test pages with multi-select configurations |
| vizro-core/tests/e2e/vizro/dashboards/default/dashboard.py | Integrated new test pages into dashboard |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def table_ag_grid_checkbox_path_by_row(table_id, row_index): | ||
| """Path to AG Grid table checkbox input by row index.""" | ||
| return f"div[id='{table_id}'] div[row-index='{row_index}'] input.ag-checkbox-input" |
Copilot
AI
Jan 27, 2026
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.
There is an extra space in the CSS selector between the two spaces before 'input.ag-checkbox-input'. This should likely be a single space for consistent CSS selector formatting.
| return f"div[id='{table_id}'] div[row-index='{row_index}'] input.ag-checkbox-input" | |
| return f"div[id='{table_id}'] div[row-index='{row_index}'] input.ag-checkbox-input" |
| gapminder = px.data.gapminder() | ||
|
|
||
|
|
Copilot
AI
Jan 27, 2026
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 variable 'gapminder' is defined but never used in this file. Consider removing it to keep the code clean.
| gapminder = px.data.gapminder() |
| selector=table_ag_grid_cell_path_by_row( | ||
| cnst.TABLE_SET_CONTROL_MULTI_SELECT, row_index=2, col_id="sepal_length" | ||
| ), | ||
| key=Keys.COMMAND, |
Copilot
AI
Jan 27, 2026
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 test uses Keys.COMMAND which is Mac-specific. For cross-platform compatibility, consider using Keys.CONTROL on Windows/Linux or detecting the operating system. This test may fail on non-Mac systems where AgGrid expects CTRL for multi-select rather than CMD.
| selector=table_ag_grid_cell_path_by_row( | ||
| cnst.TABLE_SET_CONTROL_MULTI_SELECT, row_index=0, col_id="sepal_length" | ||
| ), | ||
| key=Keys.COMMAND, |
Copilot
AI
Jan 27, 2026
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 test uses Keys.COMMAND which is Mac-specific. For cross-platform compatibility, consider using Keys.CONTROL on Windows/Linux or detecting the operating system. This test may fail on non-Mac systems where AgGrid expects CTRL for multi-select rather than CMD.
|
|
||
|
|
||
| def scatter_point_path(graph_id, point_number, trace_index=2): | ||
| return f"div[id='{graph_id}'] g[class^='trace']:nth-of-type({trace_index}) path:nth-of-type({point_number}" |
Copilot
AI
Jan 27, 2026
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.
Missing closing parenthesis in the return statement. The f-string is not properly closed.
| return f"div[id='{graph_id}'] g[class^='trace']:nth-of-type({trace_index}) path:nth-of-type({point_number}" | |
| return f"div[id='{graph_id}'] g[class^='trace']:nth-of-type({trace_index}) path:nth-of-type({point_number})" |
Description
Closes https://github.com/McK-Internal/vizro-internal/issues/2403
Notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":