Skip to content

fix: Dark mode bug, pinia store reset across Vite entry points#7851

Merged
rjsparks merged 1 commit intoietf-tools:mainfrom
holloway:global-pinia-dark-mode
Aug 23, 2024
Merged

fix: Dark mode bug, pinia store reset across Vite entry points#7851
rjsparks merged 1 commit intoietf-tools:mainfrom
holloway:global-pinia-dark-mode

Conversation

@holloway
Copy link
Contributor

The IETF meeting agenda's dark mode is broken in some configurations and it looks like this:

Screenshot from 2024-08-22 21-21-23

The bug is caused by not setting theme-dark CSS class on the <body>.

That's caused by the Pinia store theme variable being correctly set to 'dark' but then incorrectly set to null.

That was caused by two Vite entry points (main.js and embedded.js) both calling createPinia() which seems to reset the Pinia store state, restoring the theme to the default null which broke dark mode.

Fixed version:

Screenshot from 2024-08-22 21-25-37

@holloway holloway requested a review from NGPixel August 22, 2024 21:12
@codecov
Copy link

codecov bot commented Aug 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.78%. Comparing base (c7f6bde) to head (5205161).
Report is 46 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #7851    +/-   ##
========================================
  Coverage   88.78%   88.78%            
========================================
  Files         296      303     +7     
  Lines       41320    41422   +102     
========================================
+ Hits        36687    36778    +91     
- Misses       4633     4644    +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rjsparks rjsparks merged commit af21347 into ietf-tools:main Aug 23, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants