zulip-mobile icon indicating copy to clipboard operation
zulip-mobile copied to clipboard

Rename "Night mode" to "Dark theme"

Open alya opened this issue 3 years ago • 32 comments
trafficstars

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.

alya avatar Dec 16 '21 19:12 alya

I'll take it :)

arcadioramos avatar Dec 16 '21 22:12 arcadioramos

I've made the pr for this issue

arcadioramos avatar Dec 16 '21 22:12 arcadioramos

@arcadioramos please link the PR to the issue.

alya avatar Jan 05 '22 21:01 alya

Done @alya :)

arcadioramos avatar Jan 05 '22 22:01 arcadioramos

@alya want to confirm if the issue is still available? If yes, Please assign me.

ArchitJain1201 avatar Jan 10 '22 17:01 ArchitJain1201

@ArchitJain1201, the GitHub UI shows that this issue is assigned to arcadioramos.

chrisbobbe avatar Jan 11 '22 18:01 chrisbobbe

I will take this issue as there is no activity on open PR for more than 3 weeks.

Udit-takkar avatar Jan 28 '22 19:01 Udit-takkar

@alya Can i work on this ?

punitkashyup avatar Mar 19 '22 09:03 punitkashyup

@punitkashyup you can contribute by reviewing the open PR, or otherwise find another open issue to pick up. Thanks!

alya avatar Apr 06 '22 20:04 alya

@alya can you assign this issue to me

ruchitarpatil001 avatar Oct 21 '22 08:10 ruchitarpatil001

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.

alya avatar Oct 24 '22 06:10 alya

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:

  • the string the user sees
  • 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.

gnprice avatar Nov 03 '22 21:11 gnprice

Hi ! I will claim this issue if possible, @zulipbot claim

thaiscodafond avatar Nov 08 '22 14:11 thaiscodafond

@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!

alya avatar Nov 08 '22 15:11 alya

I want to work in this issue. please assign me

aritroCoder avatar Dec 07 '22 07:12 aritroCoder

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 avatar Dec 07 '22 08:12 aritroCoder

@aritroCoder I assigned this issue to you.

@chrisbobbe or @gnprice , any feedback on the propose approach above?

alya avatar Dec 07 '22 20:12 alya

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.

chrisbobbe avatar Dec 07 '22 21:12 chrisbobbe

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

aritroCoder avatar Dec 08 '22 04:12 aritroCoder

We store a ThemeSetting value in Redux as state.settings.theme; see the GlobalSettingsState type.

chrisbobbe avatar Dec 08 '22 04:12 chrisbobbe

Okay, I have changed that. Should I create a PR now?

We store a ThemeSetting value in Redux as state.settings.theme; see the GlobalSettingsState type.

aritroCoder avatar Dec 08 '22 09:12 aritroCoder

Sure! Thanks for your help. 🙂

chrisbobbe avatar Dec 08 '22 19:12 chrisbobbe

@chrisbobbe please be sure to review the PR

aritroCoder avatar Dec 11 '22 04:12 aritroCoder

If this issue is still open please assign it to me.

Smriti925 avatar Feb 09 '23 05:02 Smriti925

@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.

gnprice avatar Feb 15 '23 02:02 gnprice

@alya Is the issue still open? If yes, Please assign me as I wanna contribute.

arickahamed avatar Mar 16 '23 10:03 arickahamed

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.

yashsinghal2004 avatar Nov 18 '23 11:11 yashsinghal2004

We are not currently reviewing contributions to this project, as the mobile team is focused on https://github.com/zulip/zulip-flutter.

alya avatar Nov 20 '23 19:11 alya

Assign me

draunger avatar Dec 28 '23 19:12 draunger