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

Finish renaming Display settings -> Preferences #26874

Open
alya opened this issue Sep 26, 2023 · 94 comments
Open

Finish renaming Display settings -> Preferences #26874

alya opened this issue Sep 26, 2023 · 94 comments

Comments

@alya
Copy link
Contributor

alya commented Sep 26, 2023

To finish out the work in #25945, we should update the following uses of "display settings" -> "preferences":

zerver/lib/hotspots.py:            "Go to Settings to configure your notifications and display settings."
zerver/models.py:    ### Display settings. ###

as well as many other variables in the code (#26874 (comment)):

CZO thread

@zulipbot
Copy link
Member

Hello @zulip/server-onboarding members, this issue was labeled with the "area: onboarding" label, so you may want to check it out!

@Ciggzy1312
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

Welcome to Zulip, @Ciggzy1312! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

@arnavchhokra
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

@arnavchhokra This issue cannot be claimed, as someone else is already working on it. Please see our contributor guide for advice on finding an issue to work on. Thanks!

Ciggzy1312 added a commit to Ciggzy1312/zulip that referenced this issue Sep 27, 2023
Fixes zulip#26874

Finish renaming 'display settings' -> 'preferences' in the following files
zerver/lib/hotspots.py
zerver/models.py

Signed-off-by: Deepayan Mukherjee <deepayanmukherjee1312@gmail.com>
@Ciggzy1312
Copy link
Collaborator

PR #26882 solves this issue

Ciggzy1312 added a commit to Ciggzy1312/zulip that referenced this issue Sep 27, 2023
Fixes zulip#26874

Finish renaming 'display settings' -> 'preferences'.

Signed-off-by: Deepayan Mukherjee <deepayanmukherjee1312@gmail.com>
@timabbott
Copy link
Sponsor Member

There's a lot more things that can be renamed. update_display_settings is part of the API (and already deprecated and to be removed) and thus cannot be simply renamed, but there's a bunch more that can be done:

$ git grep -i 'display[ _-]settings' | grep -v update_display_settings | grep -v display_settings_legacy | grep -v locale
api_docs/changelog.md:  `demote_inactive_streams` display settings.
docs/overview/changelog.md:  view can now be configured via "Display settings".
docs/overview/changelog.md:- Added nice UI for selecting a default language to display settings.
web/e2e-tests/settings.test.ts:    const display_settings_section = '[data-section="preferences"]';
web/e2e-tests/settings.test.ts:    await page.click(display_settings_section);
web/e2e-tests/settings.test.ts:    await page.waitForSelector(display_settings_section, {visible: true});
web/e2e-tests/settings.test.ts:    await page.click(display_settings_section);
web/e2e-tests/settings.test.ts:    await page.waitForSelector(display_settings_section, {visible: true});
web/e2e-tests/settings.test.ts:    await page.click(display_settings_section);
web/src/admin.js:        display_settings: settings_config.get_all_display_settings(),
web/src/hashchange.js:    if (section === "display-settings") {
web/src/hashchange.js:        // Since display-settings was deprecated and replaced with preferences
web/src/hashchange.js:        // #settings/display-settings is being redirected to #settings/preferences.
web/src/server_events_dispatch.js:            const user_display_settings = [
web/src/server_events_dispatch.js:            if (user_display_settings.includes(event.property)) {
web/src/settings.js:        ...settings_config.display_settings_labels,
web/src/settings.js:        display_settings: settings_config.get_all_display_settings(),
web/src/settings_config.ts:        user_display_settings: string[];
web/src/settings_config.ts:export const get_all_display_settings = (): DisplaySettings => ({
web/src/settings_config.ts:        user_display_settings: [
web/src/settings_config.ts:export const display_settings_labels = {
web/src/settings_config.ts:    ...display_settings_labels,
web/src/settings_display.js:        const $spinner = $container.find(".emoji-display-settings-status").expectOne();
web/src/settings_display.js:                    $container.find(".emoji-display-settings-status").expectOne(),
web/src/settings_display.js:    const $spinner = $(settings_panel.container).find(".emoji-display-settings-status");
web/styles/settings.css:.display-settings-form,
web/styles/settings.css:label.display-settings-radio-choice-label {
web/styles/settings.css:    .display-settings-form select {
web/templates/settings/display_settings.hbs:<form class="display-settings-form">
web/templates/settings/display_settings.hbs:    <div class="emoji-display-settings {{#if for_realm_settings}}settings-subsection-parent{{else}}subsection-parent{{/if}}">
web/templates/settings/display_settings.hbs:            {{> settings_save_discard_widget section_name="emoji-display-settings" show_only_indicator=(not for_realm_settings) }}
web/templates/settings/display_settings.hbs:                    <label class="display-settings-radio-choice-label">
web/templates/settings/display_settings.hbs:                    <label class="display-settings-radio-choice-label">
web/templates/settings/display_settings.hbs:        {{#each display_settings.settings.user_display_settings}}
web/templates/settings/display_settings.hbs:              render_only=(lookup ../display_settings.render_only this)
web/templates/settings/organization_user_settings_defaults.hbs:    {{> display_settings prefix="realm_" for_realm_settings=true full_name=full_name}}
web/templates/settings/user_display_settings.hbs:    {{> display_settings prefix="user_" for_realm_settings=false}}
web/templates/settings_tab.hbs:    {{> settings/user_display_settings }}
web/tests/i18n.test.js:        display_settings: {
zerver/lib/hotspots.py:            "Go to Settings to configure your notifications and display settings."
zerver/models.py:    ### Display settings. ###

If you choose to work on this further, please rename one element at a time -- e.g. settings_display.js => settings_preferences.js would be one commit that carefully and thoroughly renames just that one thing, passing all tests, and emoji-display-settings => emoji-preferences would be another commit -- it's like 10x easier to review such a PR and we definitely won't review a PR for this work not done that way.

timabbott pushed a commit that referenced this issue Sep 27, 2023
@Ciggzy1312 Ciggzy1312 removed their assignment Sep 28, 2023
@AdityaDKale
Copy link
Collaborator

is this issue still open to claim?

@Ciggzy1312
Copy link
Collaborator

@AdityaDKale yes, feel free to take it up
Do follow the instructions mentioned by Tim on how to proceed

@AdityaDKale
Copy link
Collaborator

@zulipbot claim

@Winter3531
Copy link

@AdityaDKale No pressure, just curious if you're still working on this issue?

@AdityaDKale
Copy link
Collaborator

AdityaDKale commented Oct 2, 2023

Yes I am actually working on this issue. Thanks for concern.

@ezekielmose
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

zulipbot commented Oct 4, 2023

@ezekielmose This issue cannot be claimed, as someone else is already working on it. Please see our contributor guide for advice on finding an issue to work on. Thanks!

@AdityaDKale
Copy link
Collaborator

I don't think I have to modify web/e2e-tests/settings.test.ts file right?
Also should I modify

web/src/admin.js
web/src/hashchange.js
web/src/server_events_dispatch.js
web/src/settings.js
web/src/settings_config.ts
web/styles/settings.css
web/templates/settings/display_settings.hbs
web/templates/settings/organization_user_settings_defaults.hbs
web/templates/settings/user_display_settings.hbs
web/templates/settings_tab.hbs
web/tests/i18n.test.js
zerver/lib/hotspots.py
zerver/models.py

according to your previous given answer @timabbott ?

@zulipbot
Copy link
Member

zulipbot commented Apr 9, 2024

@ritikraj26 You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks!

@ritikraj26
Copy link
Collaborator

Working on it

@Someone12543
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

Welcome to Zulip, @Someone12543! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

@Someone12543
Copy link
Collaborator

Small update: still working on it!
Just as a heads up, this is my first claimed issue in zulip so I'm trying to catch up with the huge documentation on everything.
I really look forward to working on this but there are other projects occupying my time, so I'm only able to commit to this (pun intended) when I have free time, which is scarce, but I'm trying!

@Someone12543
Copy link
Collaborator

I have analyzed most of the instances that require changing, though a few questions have arisen...
Should I voice my concerns in here or should I open a thread in the zulip application itself?

@alya
Copy link
Contributor Author

alya commented May 7, 2024

It'll be best to discuss in the Zulip development community for more visibility, thanks!

@Someone12543
Copy link
Collaborator

Recently opened an aptly named topic in zulip's "issues" stream.
Would love to about about it!

Someone12543 added a commit to Someone12543/zulip that referenced this issue May 16, 2024
I started by identifying the different types of "display_settings"
and their references, all according to an earlier message in this
issue (zulip#26874) from Tim Abbot.

After analyzing the code for a solid few hours I determined that
there were 3:

a constant variable defined in server_events_dispatch.js;
a few dictionaries in different files along with a filename;
and a file named user_display_settings.hbs.

Regarding this specific commit, I just had find all instances
of the user_display_settings variable that wasn't
the user_display_settings.hbs, and since all those instances were
inside the web folder, I didn't have to worry about coupling issues,
so all instances were simply changed to user_preferences.
@Someone12543
Copy link
Collaborator

@zulipbot abandon

@nimi11
Copy link
Collaborator

nimi11 commented Jun 10, 2024

@zulipbot claim

@zulipbot
Copy link
Member

Hello @nimi11, it looks like you've currently claimed 1 issue in this repository. We encourage new contributors to focus their efforts on at most 1 issue at a time, so please complete your work on your other claimed issues before trying to claim this issue again.

We look forward to your valuable contributions!

@asttle
Copy link
Collaborator

asttle commented Jun 15, 2024

@zulipbot claim

@zulipbot
Copy link
Member

Welcome to Zulip, @asttle! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

@asttle
Copy link
Collaborator

asttle commented Jun 17, 2024

@zulipbot abandon

@RitvikaSavanna
Copy link
Collaborator

@zulipbot claim

@zulipbot
Copy link
Member

Welcome to Zulip, @RitvikaSavanna! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment