Skip to content
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

feat(/lib/components/flowbite): remove usePreferences #582

Merged
merged 6 commits into from
Feb 21, 2023

Conversation

haron68
Copy link
Contributor

@haron68 haron68 commented Feb 5, 2023

Found an issue where usePreferences when set to true overrides dark = false in theme props which is a confusing developer experience

fix #581, fix #565

Description

Removed usePreferences and use of localStorage in useThemeMode hook for managing dark theme state in the ThemeContext

Fixes #581, #565

Type of change

Bug fix and refactor

Breaking changes

  • Removed usePreferences from ThemeProp
  • Removed use of local storage in useThemeMode hook

How Has This Been Tested?

  1. Set your OS level to dark mode
  2. Run locally yarn start
  3. In src/index.tsx set dark ThemeProp as dark: false
<Flowbite theme={{ theme, dark: false }}>
    <BrowserRouter>
    <Root />
  </BrowserRouter>
</Flowbite>
  1. Observe that the documentation site is now light

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

…rk = false

Found an issue where userpreferences when set to true overrides dark = false in theme props which is
a confusing developer experience

fix themesberg#581
Fixes bug with dark = false not working. Removed usePreferences because of potential legal issues
due to GDPR

BREAKING CHANGE: ThemeProps no longer includes usePreferences

fix themesberg#565, fix themesberg#581
@haron68 haron68 changed the title fix(flowbite): fix bug with user preferences overriding dark = false fix(theme): remove usePreferences from ThemeProps Feb 6, 2023
Refactored useThemeMode to fetch context state
@codecov
Copy link

codecov bot commented Feb 13, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@e88cdab). Click here to learn what that means.
Patch coverage: 90.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #582   +/-   ##
=======================================
  Coverage        ?   99.31%           
=======================================
  Files           ?      131           
  Lines           ?     6387           
  Branches        ?      477           
=======================================
  Hits            ?     6343           
  Misses          ?       44           
  Partials        ?        0           
Impacted Files Coverage Δ
src/lib/components/Flowbite/ThemeContext.tsx 87.50% <80.00%> (ø)
src/lib/components/Flowbite/Flowbite.tsx 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tulup-conner tulup-conner marked this pull request as draft February 21, 2023 03:36
@tulup-conner
Copy link
Collaborator

tulup-conner commented Feb 21, 2023

That is a confusing DX for sure. I don't think it makes sense to destroy the existing API though. Can we change the behavior to just have dark take precedence over usePreferences?

Never mind, I see this has actually been discussed already elsewhere, see #565

@tulup-conner tulup-conner added the 🚀 enhancement New feature or request label Feb 21, 2023
@tulup-conner tulup-conner changed the title fix(theme): remove usePreferences from ThemeProps fix(/lib/components/flowbite): respect dark mode before usePreferences Feb 21, 2023
@tulup-conner tulup-conner changed the title fix(/lib/components/flowbite): respect dark mode before usePreferences feat(/lib/components/flowbite): remove usePreferences Feb 21, 2023
@tulup-conner tulup-conner self-assigned this Feb 21, 2023
@tulup-conner tulup-conner marked this pull request as ready for review February 21, 2023 03:46
src/lib/components/Flowbite/Flowbite.tsx Outdated Show resolved Hide resolved
src/lib/components/Flowbite/ThemeContext.tsx Outdated Show resolved Hide resolved
# Conflicts:
#	src/docs/pages/ThemePage.tsx
#	src/lib/components/Flowbite/ThemeContext.tsx
improve readability and fix merge conflicts. Update documentation for ThemePage

themesberg#565
Since we're no longer using localStorage no need to hard check for false
@haron68 haron68 requested a review from tulup-conner February 21, 2023 04:59
@tulup-conner
Copy link
Collaborator

LGTM, can you double check this @rluders

@tulup-conner tulup-conner requested a review from rluders February 21, 2023 05:42
Copy link
Collaborator

@rluders rluders left a comment

Choose a reason for hiding this comment

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

Looks good. Let's merge it. :)

@rluders rluders merged commit 77cbc27 into themesberg:main Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 enhancement New feature or request
Projects
None yet
3 participants