Skip to content

feat(theme): Initial theme toggle functionality#36

Draft
websterbontilao wants to merge 5 commits intomainfrom
theme-update
Draft

feat(theme): Initial theme toggle functionality#36
websterbontilao wants to merge 5 commits intomainfrom
theme-update

Conversation

@websterbontilao
Copy link
Collaborator

Changes

Related issues

Fixes #___

Checklist

  • PR is ready for review (if not, it should be a draft).
  • PR title follows Conventional Commits guidelines.
  • Screenshots/video added.
  • Tests added.
  • Self-review done.

@vercel
Copy link

vercel bot commented Sep 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
route-planner ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2023 11:21am

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be removed, since we'll utilize mapbox theming instead of open street map

Comment on lines +8 to +13
<ThemeContextProvider>
<AppContextProvider>
<Wrapper />
<Drawer />
</AppContextProvider>
</ThemeContextProvider>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please place the logic for switching themes in AppContextProvider instead of creating a new context provider ThemeContextProvider

Suggested change
<ThemeContextProvider>
<AppContextProvider>
<Wrapper />
<Drawer />
</AppContextProvider>
</ThemeContextProvider>
<AppContextProvider>
<ThemeProvider theme={/* insert theme value from app context here */} >
<Wrapper />
<Drawer />
</ThemeProvider>
</AppContextProvider>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants