Set the map language to UI language in new branch #797
Conversation
…ting invalid. And use zoom:1 to show the global map.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #797 +/- ##
==========================================
- Coverage 65.37% 65.29% -0.09%
==========================================
Files 138 138
Lines 12891 12951 +60
Branches 533 533
==========================================
+ Hits 8428 8456 +28
- Misses 4303 4339 +36
+ Partials 160 156 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I've merged this PR into the #910 - a temporary alternative to the |
|
I really like this addition, and I think it would be a good idea to merge it. But I think we recently added Ukrainian which should probably be added before we can merge this. |
Oh, didn't expect that there might be an ordering issue (and GitHub doesn't show any conflicts with master now). Do you see an issue with merging this PR after that commit is done? |
|
@kkovaletp Merge conflicts unfortunatly don't work in that way. A merge conflict happens when 2 people edit the same area of code, in this circumstance neither have modified the same area. @WindLi001 I have added a comment to make this more generic and add a tie for this to the language selection using a map which should make it so that people adding languages in the future are less likely to miss it. A test could be written for this to ensure the |
| return | ||
| } | ||
|
|
||
| switch (map_language) { |
There was a problem hiding this comment.
We already have all of this information within the Userpreferences.tsx called languagePreferences. We should extract this and the elements in languagePreferences into a map within this file which is LanguageTranslation -> languageCode Although in this case it does appear that the Chinese traditional & simplified do not use the same code as we do within UserPreferences (You might need an object rather than one field in the map).
Rewriting it in this way gives us somewhere more central to pick up on language codes and means things don't get missed in the future. A test can also be written to ensure the language selection list length is the same as the map to stop someone doing something strange in the future.
|
@jordy2254 as the PR owner is not responding, but @viktorstrate and I think that this improvement is adding value to the Photoview, Can you please fork it to your repo (by a command, similar to BTW, this PR is 1 of a few still not merged from the #910, so we're close to our 1st milestone) |
|
I just found the #672, where there is the link to the MapBox example, so copy it here just in case: https://docs.mapbox.com/mapbox-gl-js/example/language-switch/ |
|
@vl-ivanov, please review this PR. |
|
@kkovaletp there are multiple unresolved issues that most likely the original PR author will not implement, since there are no comments from @WindLi001 after creating the PR.
|
Dismissing approval because of the list of required changes
This is a solution of #789. The language of map in Place will be set to the same one of the UI. If the UI language is NOT supported by Mapbox, then the English map will be provided.
This is a new pull request of Set the map language to UI language, and I commit it to a new branch other than master. The old one will be closed soon.
As the comment in the old pull request #790, I delete the setting of zoom and center, and delete the setting of projection, which make the map language cannot be changed.
I have a look to the commit setting the projection, it said "chore: add mapbox globe view", so I guess the committer just want to show the mapbox with whole world map initially. But I think maybe he or she don't know what the projection really means, in fact global projection just make a spherical map. So, I add zoom: 1 to show the mapbox with whole world map initially.