Fix #789: Set the map language to UI language #790
Conversation
Codecov Report
@@ Coverage Diff @@
## master #790 +/- ##
==========================================
- Coverage 65.51% 65.24% -0.27%
==========================================
Files 137 137
Lines 12903 12965 +62
Branches 533 533
==========================================
+ Hits 8453 8459 +6
- Misses 4295 4351 +56
Partials 155 155
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Adding another bug fix. Now photoview generating 1024px thumbnail for each photo, including the photo smaller than 1024px. |
| @@ -59,13 +60,17 @@ const useMapboxMap = ({ | |||
| mapboxLibrary.accessToken = mapboxData.mapboxToken | |||
|
|
|||
| map.current = new mapboxLibrary.Map({ | |||
| zoom: 6, // zoom must be initialized to 6 or more, or the language of map cannot be set. | |||
| center: [-77.0259, 38.901], // Initial to the land. | |||
There was a problem hiding this comment.
Why did you hard-code these coordinates in particular?
There was a problem hiding this comment.
I set the map center, because if it isn't set, the center will be in the ocean by default. Then at zoom 6, the clients will see nothing on the map, and maybe they will think the map is broken. The best way is choosing the coordinate of the client's nation by default, but I don't find the way to get this information. So I have to hard-code a random coordinate in the land.
There was a problem hiding this comment.
Is it maybe appropriate to use the default [0, 0] coordinate and then have the default zoom be zoomed out to the max value so the entire world is visible and the user can zoom to their desired place?
There was a problem hiding this comment.
I set zoom to 6 because of a bug of mapbox-gl-language. https://github.com/mapbox/mapbox-gl-language/issues/60
In fact, at beginning I don't set either the zoom or the center, but just only call MapboxLanguage({defaultLanguage: xxx}). Then I found the map language is still English. I check the codes several times but have no idea. Fortunately, at last I found the BUG of mapbox-gl-language and raise the issue above. But until now, this bug is still here.
So now we have 2 choices:
- Waiting mapbox-gl-language fix this bug, and don't set either the zoom or the center. But before they fix it, the map language will always be English.
- Setting the zoom and center to provide multi-language map, waiting them fix this bug, and then delete the setting of zoom and center.
I want to fix this bug by myself, but I'm still learning JavaScript now.
There was a problem hiding this comment.
With the beginner’s luck, I found another way can change the map language without setting zoom to more than 6. If delete the setting of map projection in ui\src\Pages\PlacesPage\PlacesPage.tsx like this,
const { mapContainer, mapboxMap, mapboxToken } = useMapboxMap({
configureMapbox: configureMapbox({ mapboxData, dispatchMarkerMedia }),
mapboxOptions: {
//projection: {
// name: 'globe',
//},
},
})
the map language can be changed too.
I still don't know why the map language is related to the zoom or projection. And I have a look to the code of mapbox-gl-language but found nothing about zoom or projection.
I think may the map projection is important to the plane and ship, but not a photo gallery :-). So, if you agree, I'll revert this commit, delete the setting of zoom and center, delete the setting of projection, and commit again.
There was a problem hiding this comment.
I move this commit to a new pull request #797, and this one will be closed.
| if width > dimensions.Width { | ||
| width = dimensions.Width | ||
| height = dimensions.Height | ||
| } |
There was a problem hiding this comment.
I'd appreciate if you send another PR for this change.
Should it also consider if the height is smaller?
There was a problem hiding this comment.
Because the aspect of thumbnail is the same as photo, comparing width or height is enough. I choose comparing width.
There was a problem hiding this comment.
I move this commit to a new pull request #798, and this one will be closed.
This is a solution of photoview#789. The language of the map in Place will be set to the same as 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 setting 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 per the comment in the old pull request photoview#790, I deleted the settings of zoom and center and deleted the setting of projection, which means the map language cannot be changed. I have looked at the commit setting the projection, it said "chore: add mapbox globe view", so I guess the committer just wants to show the mapbox with the whole world map initially. But I think maybe he or she doesn't know what the projection really means, in fact, global projection just makes a spherical map. So, I add zoom: 1 to show the mapbox with the whole world map initially. --------- Co-authored-by: WindLi001 <lichenggang1@hotmail.com>
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.