Open alya opened 2 years ago
I'll take it :)
I've made the pr for this issue
@arcadioramos please link the PR to the issue.
Done @alya :)
@alya want to confirm if the issue is still available? If yes, Please assign me.
@ArchitJain1201, the GitHub UI shows that this issue is assigned to arcadioramos
.
I will take this issue as there is no activity on open PR for more than 3 weeks.
@alya Can i work on this ?
@punitkashyup you can contribute by reviewing the open PR, or otherwise find another open issue to pick up. Thanks!
@alya can you assign this issue to me
I removed the "good first issue" and "help wanted" labels, as we expect this issue to be solved via solving #4009.
@BigHeadCreations and others, thanks for your work on this! I recommend finding another issue to pick up.
We've just merged part of #5527, which was aimed at #4009. We didn't end up merging the main commit, because of an RN bug that came up. That RN bug means that solving #4009 will involve more complexity than we'd expected.
As a result, this is still open and good for someone to pick up.
We did in #5527, specifically 43acc4199, take care of part of this renaming. The remaining bits are:
ThemeSetting
type -- note we'll need a migration (https://github.com/zulip/zulip-mobile/blob/main/docs/howto/new-feature.md#redux-state-migrations) when changing that, because it's something we storegit grep -i night
to find them.Hi ! I will claim this issue if possible, @zulipbot claim
@thaiscodafond Zulipbot is not active in the mobile repository.
Please post a comment describing your proposed approach when you're ready. I can assign the issue to you once you have a rough plan. Thanks!
I want to work in this issue. please assign me
Plan of working: In src/settings/SettingsScreen.js
I have to change the line <SwitchRow label="Night mode" value={theme === 'night'} onValueChange={handleThemeChange} />
to <SwitchRow label="Dark theme" value={theme === 'night'} onValueChange={handleThemeChange} />
.
A better approach would be to scan the entire code base for the text Night mode
and replace all with Dark theme
as there are also translation files in this app, and comments in some places that reference this as Night mode
@aritroCoder I assigned this issue to you.
@chrisbobbe or @gnprice , any feedback on the propose approach above?
Thanks, @aritroCoder! The change you describe in SettingsScreen.js will change
- the string the user sees
but the issue also requires two more changes, as Greg mentioned above in https://github.com/zulip/zulip-mobile/issues/5169#issuecomment-1302690629:
- the
ThemeSetting
type -- note we'll need a migration (https://github.com/zulip/zulip-mobile/blob/main/docs/howto/new-feature.md#redux-state-migrations) when changing that, because it's something we store- a few remaining occurrences in the code and comments -- try
git grep -i night
to find them.
Thanks, @aritroCoder! The change you describe in SettingsScreen.js will change
- the string the user sees
but the issue also requires two more changes, as Greg mentioned above in #5169 (comment):
- the
ThemeSetting
type -- note we'll need a migration (https://github.com/zulip/zulip-mobile/blob/main/docs/howto/new-feature.md#redux-state-migrations) when changing that, because it's something we store- a few remaining occurrences in the code and comments -- try
git grep -i night
to find them.
Thanks for the suggestions @chrisbobbe!
I did not find any entry for ThemeSetting
in src/storage/migrations.js
for the migration thing (or will I have to add it?) . But I changed export type ThemeSetting = 'default' | 'night';
to export type ThemeSetting = 'default' | 'dark';
in src/reduxTypes.js
.
I also have changed the references in the remaining code and comments from night
to dark
, wherever it referenced the dark theme
We store a ThemeSetting
value in Redux as state.settings.theme
; see the GlobalSettingsState
type.
Okay, I have changed that. Should I create a PR now?
We store a
ThemeSetting
value in Redux asstate.settings.theme
; see theGlobalSettingsState
type.
Sure! Thanks for your help. 🙂
@chrisbobbe please be sure to review the PR
If this issue is still open please assign it to me.
@Smriti925 Please take a look at our guidelines for picking an issue to work on: https://zulip.readthedocs.io/en/latest/contributing/contributing.html#picking-an-issue-to-work-on Before trying to claim an issue, please spend some time looking through the code to find what will need to change, and work out how you'd approach the problem.
@alya Is the issue still open? If yes, Please assign me as I wanna contribute.
Hey @alya I wanted to know if this issue is active or not and if not please assign me good first issue which is active as I want to start my contribution in Zulip org.
We are not currently reviewing contributions to this project, as the mobile team is focused on https://github.com/zulip/zulip-flutter.
Assign me
In https://github.com/zulip/zulip/issues/20228, we renamed "night mode" to "dark theme" in the web app and the Help center. We should make the mobile app theme name consistent with the web app as well.