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

Rename "Night mode" to "Dark theme" #5169

Open
alya opened this issue Dec 16, 2021 · 29 comments · Fixed by aritroCoder/zulip-mobile#3, aritroCoder/zulip-mobile#4 or aritroCoder/zulip-mobile#6 · May be fixed by #5595
Open

Rename "Night mode" to "Dark theme" #5169

alya opened this issue Dec 16, 2021 · 29 comments · Fixed by aritroCoder/zulip-mobile#3, aritroCoder/zulip-mobile#4 or aritroCoder/zulip-mobile#6 · May be fixed by #5595
Assignees
Labels
a-settings The settings screen, syncing settings with the server, etc good first issue help wanted webapp parity Features that exist in the webapp that we need to port to mobile. We aren't aiming for full parity.

Comments

@alya
Copy link
Collaborator

alya commented Dec 16, 2021

In zulip/zulip#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 alya added help wanted good first issue a-settings The settings screen, syncing settings with the server, etc webapp parity Features that exist in the webapp that we need to port to mobile. We aren't aiming for full parity. labels Dec 16, 2021
@arcadioramos
Copy link

I'll take it :)

@arcadioramos
Copy link

I've made the pr for this issue

@alya
Copy link
Collaborator Author

alya commented Jan 5, 2022

@arcadioramos please link the PR to the issue.

@arcadioramos
Copy link

Done @alya :)

@ArchitJain1201
Copy link

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

@chrisbobbe
Copy link
Contributor

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

@Udit-takkar
Copy link

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

@punitkashyup
Copy link

@alya Can i work on this ?

@alya
Copy link
Collaborator Author

alya commented Apr 6, 2022

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

@ruchitarpatil001
Copy link

@alya can you assign this issue to me

@alya
Copy link
Collaborator Author

alya commented Oct 24, 2022

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.

@gnprice
Copy link
Member

gnprice commented Nov 3, 2022

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 43acc41, take care of part of this renaming. The remaining bits are:

@thaiscodafond
Copy link

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

@alya
Copy link
Collaborator Author

alya commented Nov 8, 2022

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

@aritroCoder
Copy link

I want to work in this issue. please assign me

@aritroCoder
Copy link

aritroCoder commented Dec 7, 2022

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

@alya
Copy link
Collaborator Author

alya commented Dec 7, 2022

@aritroCoder I assigned this issue to you.

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

@chrisbobbe
Copy link
Contributor

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):

@aritroCoder
Copy link

aritroCoder commented Dec 8, 2022

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):

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

@chrisbobbe
Copy link
Contributor

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

@aritroCoder
Copy link

aritroCoder commented Dec 8, 2022

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.

@chrisbobbe
Copy link
Contributor

Sure! Thanks for your help. 🙂

@aritroCoder
Copy link

@chrisbobbe please be sure to review the PR

aritroCoder added a commit to aritroCoder/zulip-mobile that referenced this issue Dec 15, 2022
Fixes: zulip#5169
Changed night mode to dark theme. Added tests, migrations, and updated comments.
aritroCoder added a commit to aritroCoder/zulip-mobile that referenced this issue Dec 15, 2022
Fixes: zulip#5169
Changed night mode to dark theme. Added tests, migrations, and updated comments.
aritroCoder added a commit to aritroCoder/zulip-mobile that referenced this issue Dec 18, 2022
Fixes: zulip#5169
Changed night mode to dark theme. Added tests, migrations, and updated comments.
aritroCoder added a commit to aritroCoder/zulip-mobile that referenced this issue Dec 23, 2022
Fixes: zulip#5169
Changed night mode to dark theme. Added tests, migrations, and updated comments.
@Smriti925

This comment was marked as off-topic.

@gnprice

This comment was marked as off-topic.

@arickahamed

This comment was marked as off-topic.

@yashsinghal2004
Copy link

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.

@alya
Copy link
Collaborator Author

alya commented Nov 20, 2023

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

@draunger

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-settings The settings screen, syncing settings with the server, etc good first issue help wanted webapp parity Features that exist in the webapp that we need to port to mobile. We aren't aiming for full parity.
Projects
None yet