Implement persistent dark mode setting and & Implement persistent dark mode setting #2423
+37
−56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changed the mechanism for retrieving the user's selected language. Instead of directly setting the language in the i18n configuration, the updated code now fetches the short name of the language based on the savedLanguage value. This short name is then used to persistently set the user's preferred language across platform sessions, ensuring that the language preference is retained even after a refresh. This update addresses the issue where the platform would revert to English on every refresh, disregarding the user's previously selected language preference.
dded functionality to save the dark mode state in the user's local storage. This ensures that the dark mode preference is maintained across different sessions and page reloads. When the user selects dark mode, this preference is now stored locally, and upon returning to the platform, the dark mode setting is automatically applied based on the saved preference. This change enhances user experience by maintaining consistent appearance settings, eliminating the need to re-enable dark mode on each visit.
dosubot
bot
added
size:S
|
Thanks a lot! We will check that tomorrow :) |
Zewed
reviewed
Apr 12, 2024
frontend/lib/context/UserSettingsProvider/User-settings.provider.tsx
Outdated
Show resolved
Hide resolved
frontend/lib/context/UserSettingsProvider/User-settings.provider.tsx
Outdated
Show resolved
Hide resolved
frontend/app/user/components/LanguageSelect/hooks/useLanguageHook.ts
Outdated
Show resolved
Hide resolved
frontend/app/user/components/LanguageSelect/hooks/useLanguageHook.ts
Outdated
Show resolved
Hide resolved
|
@ellipsis-dev Please review this pr |
|
OK! Reviewing this PR... Responding to this comment by @StanGirard. For more information about Ellipsis, check the documentation. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me!
- Reviewed the entire pull request up to bb45566
- Looked at
46lines of code in2files - Took 1 minute and 55 seconds to review
More info
- Skipped
0files when reviewing. - Skipped posting
2additional comments because they didn't meet confidence threshold of50%.
1. /frontend/lib/context/UserSettingsProvider/User-settings.provider.tsx:24:
- Assessed confidence :
80% - Grade:
30% - Comment:
Consider providing a more sensible default value forisDarkModewhenwindowis undefined. Currently, it defaults totruewhich might not always be the desired behavior. - Reasoning:
In the UserSettingsProvider, the initial state of isDarkMode is set based on the value stored in localStorage. However, if the window object is undefined (which might be the case during server-side rendering), the initial state is always set to true. This might not be the desired behavior, as it assumes that the user always prefers dark mode when server-side rendering is used. It would be better to have a more sensible default, or to delay the rendering of the component until the client-side scripts have loaded and the actual preference can be determined.
2. /frontend/app/user/components/LanguageSelect/hooks/useLanguageHook.ts:62:
- Assessed confidence :
100% - Grade:
0% - Comment:
Consider adding a check to ensure that the saved language exists in the list of available languages. If the saved language is not in the list, you might want to set the current language to a default value. - Reasoning:
In the useLanguageHook, the saved language is retrieved from localStorage and then used to set the current language. However, there is no check to ensure that the saved language actually exists in the list of available languages. If the saved language is not in the list, the current language will be set to undefined, and an error might occur when trying to access the shortName property of undefined.
Workflow ID: wflow_8ci9kLA9Uc4YR5dX
Not what you expected? You can customize the content of the reviews using rules. Learn more here.
dosubot
bot
added
size:XL
dosubot
bot
added
size:M
|
@StanGirard Just fixed all the problems |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Checklist before requesting a review
Please delete options that are not relevant.
Screenshots (if appropriate):
Summary:
This PR updates the
useLanguageHookandUserSettingsProviderfunctions to use theshortNameof the selected language when changing the language and to retrieve and store the dark mode setting from and to local storage, respectively.Key points:
useLanguageHookin/frontend/app/user/components/LanguageSelect/hooks/useLanguageHook.tsto useshortNameof selected language when changing language.UserSettingsProviderin/frontend/lib/context/UserSettingsProvider/User-settings.provider.tsxto retrieve dark mode setting from local storage.changeevent onmediaQueryListinUserSettingsProviderto store updated dark mode setting in local storage.Generated with ❤️ by ellipsis.dev