Added accesibility for default system theme to p5.js-web-editor#3371
Added accesibility for default system theme to p5.js-web-editor#3371takshittt wants to merge 11 commits into
Conversation
|
To check the checkbox you can add |
|
Reviewed and tested the implementation of the default system theme. Verified that theme switches correctly based on system preferences across supported browsers. Ready for review and merge. |
|
Thanks so much for working on this! I just checked it out and can confirm that it seems to work so far! I'm not sure if this might've been mentioned somewhere already, but could you delve into why |
|
The migration from react-helmet to react-helmet-async was primarily done for testing purposes, but not exclusively. It also helps maintain consistency between the testing and production environments. However, I realize this is a broader change that could affect the overall code workflow. I'm happy to switch back to react-helmet if you prefer. |
|
Thanks for providing the additional context! In the React 19 docs, there's a section regarding Metadata, which provides more native support but also recommends using a supplementary library like As a note and reference to the work here so far, if we do decide to switch to a different metadata library, I think we could move away from using a utility file like That said, I do think the inclusion of |
|
I've replaced react-helmet-async with react-helmet to simplify the setup and reduce dependencies. |
raclim
left a comment
There was a problem hiding this comment.
Thanks so much for the update on this! I added in a few more notes/questions, but overall may look good to go soon!
|
Hi @raclim |
|
I think this overall looks good to me! The only remnant issue seems to be merge conflicts with the |
I added a button in the settings dropdown menu as "system preferences" which enables default system theme of the user to the p5.js web editor.
Fixes #3061
Changes: added accessibility for default system theme in the p5.js-web-editor
I have verified that this pull request:
npm run lint)npm run test)developbranch.Fixes #3061