Skip to content

Add language setting #1461

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

AshutoshKhadse23
Copy link

Issue #1139

Added the language setting into the appropriate setting button.

issue.1139.mp4

@gnprice
Copy link
Member

gnprice commented Apr 3, 2025

Hi, thanks for the contribution.

Please see our README for the basic requirements this PR will need to meet before we can review it.

@AshutoshKhadse23
Copy link
Author

Did you mean to add test for the language code ?

@gnprice
Copy link
Member

gnprice commented Apr 3, 2025

What does the README say under "Submitting a pull request"?

@AshutoshKhadse23
Copy link
Author

Understood !!

@AshutoshKhadse23
Copy link
Author

Added the test which ensure that :

  • Users can view and change language settings
  • The UI properly reflects the current language
  • Navigation works correctly
  • The search functionality is present
  • Selection state is visually indicated

@gnprice
Copy link
Member

gnprice commented Apr 3, 2025

Yeah, tests are one thing that's needed. There are two bulleted list items in that section, though — what's the other one say?

This also doesn't appear to actually add any setting. A setting means something the user sets which is then stored, on the device, so that their choice is remembered the next time they run the app. See the issue, and the broader issue it links to which this issue is "an instance of".

@AshutoshKhadse23
Copy link
Author

Sorry for that I will add the relevant test case for that and now I had store the language preference.

@PIG208 PIG208 self-assigned this Apr 15, 2025
@gnprice gnprice added maintainer review PR ready for review by Zulip maintainers and removed maintainer review PR ready for review by Zulip maintainers labels Apr 16, 2025
@PIG208
Copy link
Member

PIG208 commented Apr 22, 2025

Hi @AshutoshKhadse23! Thanks for the PR. Are you still working on this?

@AshutoshKhadse23
Copy link
Author

Hello @PIG208 ,
I think this is the final code, but whyis this not able to be understood "This branch cannot be rebased due to conflicts" as the branch is updated and no conflict (think so ).

@PIG208
Copy link
Member

PIG208 commented Apr 24, 2025

What does the README say under "Submitting a pull request"?

Yeah, tests are one thing that's needed. There are two bulleted list items in that section, though — what's the other one say?

Please refer to the other item Greg referred to in his previous comment. That might help with solving this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants