Custom Dark Theme - section updates & e2e tests#12798
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
mayagbarnes
changed the title
mayagbarnes
added
security-assessment-completed
impact:users
Graphite App
📉 Frontend coverage change detectedThe frontend unit test (vitest) coverage has decreased by 0.0000%
✅ Coverage change is within normal range. |
mayagbarnes
changed the title
mayagbarnes
changed the base branch from
custom-dark-theme-fe-3
to
graphite-base/12798
mayagbarnes
changed the base branch from
graphite-base/12798
to
custom-dark-theme-fe-3
mayagbarnes
added
change:chore
sfc-gh-mbarnes
force-pushed
the
custom-dark-theme-fe-4
branch
2 times, most recently
from
af33031 to
344d8ec
Compare
sfc-gh-mbarnes
force-pushed
the
custom-dark-theme-fe-3
branch
from
5c5d49e to
fca5453
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive e2e tests for custom dark theme configurations, testing theme precedence between [theme], [theme.light], [theme.dark], and their sidebar-specific variants.
Key Changes
- Custom light theme e2e tests: Tests theme precedence between
[theme]and[theme.light]configurations with both auto and explicit theme selection - Custom dark theme e2e tests: Tests theme precedence between
[theme]and[theme.dark]configurations with browser context set to dark mode preference - Sidebar-specific theme tests: Tests complex precedence rules between general theme configs, sidebar configs, and mode-specific sidebar configs
Reviewed Changes
Copilot reviewed 8 out of 50 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
e2e_playwright/theming/custom_light_theme_test.py |
Light theme e2e tests with fixture for theme configuration and tests for auto/explicit theme selection |
e2e_playwright/theming/custom_light_theme.py |
Streamlit app script for light theme testing that imports shared theme tester |
e2e_playwright/theming/custom_light_sidebar_theme_test.py |
Light sidebar theme tests with complex precedence validation for sidebar-specific configurations |
e2e_playwright/theming/custom_light_sidebar_theme.py |
App script for light sidebar theme testing |
e2e_playwright/theming/custom_dark_theme_test.py |
Dark theme e2e tests with browser context override and comprehensive theme switching validation |
e2e_playwright/theming/custom_dark_theme.py |
App script for dark theme testing |
e2e_playwright/theming/custom_dark_sidebar_theme_test.py |
Dark sidebar theme tests with multi-level configuration precedence testing |
e2e_playwright/theming/custom_dark_sidebar_theme.py |
App script for dark sidebar theme testing |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
mayagbarnes
changed the base branch from
custom-dark-theme-fe-3
to
graphite-base/12798
sfc-gh-mbarnes
force-pushed
the
custom-dark-theme-fe-4
branch
from
344d8ec to
77d6018
Compare
mayagbarnes
changed the base branch from
graphite-base/12798
to
custom-dark-theme-fe-3
mayagbarnes
changed the base branch from
custom-dark-theme-fe-3
to
graphite-base/12798
sfc-gh-mbarnes
force-pushed
the
custom-dark-theme-fe-4
branch
from
77d6018 to
3c4b3fc
Compare
mayagbarnes
changed the base branch from
graphite-base/12798
to
custom-dark-theme-fe-3
sfc-gh-mbarnes
force-pushed
the
custom-dark-theme-fe-4
branch
from
3c4b3fc to
9f347b4
Compare
mayagbarnes
changed the title
There was a problem hiding this comment.
suggestion (non-blocking): Great work on the test coverage here. From a testing pyramid perspective, run_theme_tester_app gives us valuable E2E coverage, but its snapshots can be high-maintenance. Since the core of these changes is in the JS, maybe we could test the logic with JS unit tests instead?
That would give us more focused, less brittle tests at a lower level, reserving the heavier snapshot tests for broader UI validation, of which we already have some decent coverage. What do you think about shifting the testing for this specific logic to JS unit tests? As indicated, this feedback is not blocking, but something to consider as a potential follow-up to lower the maintenance burden.
There was a problem hiding this comment.
Fair suggestion - these e2e tests were added for the full feature behavior vs. particularly this logic, but the custom theme e2e tests have become quite sprawling, so will look to consolidate in advanced theming follow ups
Describe your changes
Part 5 of Custom Dark Theme Feature
Resolves section merge issue introduced with updating to lodash merge in #12760
mergeWithinstead to handle protobuf defaultsmergeWithAdds e2e tests for
theme.lightandtheme.darkconfigurations[theme]&[theme.light/dark]configs[theme.light/dark],[theme.sidebar], &[theme.light/dark.sidebar]configsTesting Plan