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

Languages order preference #5826 #6059

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

Conversation

Adeeth101
Copy link

Description (required)

Fixes #5826
What changes did you make and why?

Very similar to my original PR, except changes made are now made to the kotlin files rather than the old java versions.

Added a new preference category in the preferences UI for secondary language picking
Prefs was also changed to hold a serialised string of secondary language codes

Strings.XML:
Added new strings that represent additions in UI

SettingsFragments (Kotlin)
Had to accommodate new preference in OnCreate
Made a new dialog called prepareSecondaryLanguagesDialog().
Made helper methods for that dialog that use a new language adapter SavedLanguagesAdapter
modified saving and getting key value methods to accommodate the new category in Prefs

NearbyController and NearbyPlaces: (Kotlin)
Injected the KV store into the controller and passed the serialised string as a new argument to the json API client in NearbyPlaces

OkHttpJsonApiClient: (Kotlin)
changed the getPlaces method to make use of the new parameter (the serialised language code)
made stringbuilders (the kolin equivalent) to automatically generate further SparQL queries based on the serialised code list
(Also modified the SparQL file itself to add placeholders for the additional strings created by the method)

Tests performed (required)
Same as last PR

Tested {build variant, e.g. ProdDebug} on {name of device or emulator} with API level {API level}.

Screenshots (for UI changes only)

image
image

Need help? See https://support.google.com/android/answer/9075928


Note: Please ensure that you have read CONTRIBUTING.md if this is your first pull request.

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since yesterday I am testing your branch. Would you know why the description was not in Slovenian for the item shown in the screencast below?

screen-20241222-173415.mp4

@Adeeth101
Copy link
Author

Is it just for slovenian or is it simply not working at all?

Ill check everything again and let you know.

@nicolas-raoul
Copy link
Member

I tried with another language and I am seeing the same issue (I tapped the refresh button to removed cached pin descriptions):

Screenshot_20241222-233218.png

Screenshot_20241222-233154.png

Screenshot_20241222-233243.png

@Adeeth101
Copy link
Author

@nicolas-raoul Just got back from work, I'll look into it today. I'm not sure why this is happening right now. In the meantime, is there an easy way of checking what SPARQL queries were being submitted?

@nicolas-raoul
Copy link
Member

Yes, all queries are visible in logcat. 🙂

@Adeeth101
Copy link
Author

Hi @nicolas-raoul can I continue this in the weekend. Im not getting any time right now.

@Adeeth101
Copy link
Author

Actually since tmr is a public holiday I can see to it.

Adith101 and others added 2 commits December 25, 2024 23:29
Also fixes the settingsFragment to select different primary and secondary description labels.
@Adeeth101
Copy link
Author

@nicolas-raoul I made some changes, could you please check again?

@Adeeth101
Copy link
Author

I think the problem was that the nearby controller was taking the Locale default language as the default description language instead of the default description stored in prefs. Also I fixed an issue with my secondary dialog setup, now it makes sure that the primary description language cannot be selected in the secondary description language selection aswell.

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing your latest commit now. Any idea why the description is in Slovenian here rather than German? Thanks! 🙂

https://m.wikidata.org/wiki/Q11378229

screen-20241229-080322.mp4

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.

Languages order preference
3 participants