Skip to content

Commit

Permalink
Fix missing map pins on rtl site (#917)
Browse files Browse the repository at this point in the history
- use ltr instead of rtl attribute in the map div since switching to rtl would disrupt the coordinate to css mapping calculated by the map providers
- adjust map boundaries based on location of the result content wrapper, which could be left or right of the viewpoint (ltr or rtl)

J=SLAP-1515
TEST=manual

- launched multilang site, see that pins on map on arabic is present and the pin locations matched with the ones on english page.
  • Loading branch information
yen-tt committed Aug 16, 2021
1 parent 9c2a179 commit 75d30ee
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 9 deletions.
2 changes: 1 addition & 1 deletion static/js/theme-map/Maps/Providers/Google.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class GoogleMap extends ProviderMap {
constructor(options) {
super(options);

const zoomControlPosition = isRTL(options.language)
const zoomControlPosition = isRTL(options.locale)
? google.maps.ControlPosition.LEFT_TOP
: google.maps.ControlPosition.RIGHT_TOP;

Expand Down
2 changes: 1 addition & 1 deletion static/js/theme-map/Maps/Providers/Mapbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class MapboxMap extends ProviderMap {
// Add the zoom control
if (options.controlEnabled) {
const zoomControl = new mapboxgl.NavigationControl({showCompass: false})
isRTL(options.language)
isRTL(options.locale)
? this.map.addControl(zoomControl, 'top-left')
: this.map.addControl(zoomControl);
}
Expand Down
14 changes: 9 additions & 5 deletions static/js/theme-map/ThemeMapConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { PinImages } from './PinImages.js';
import { ClusterPinImages } from './ClusterPinImages.js';
import { getLanguageForProvider } from './Util/helpers.js';
import { defaultCenterCoordinate } from './constants.js';
import isRTL from '../../common/rtl';

/**
* The configuration for the ThemeMap component.
Expand Down Expand Up @@ -100,8 +101,8 @@ export default class ThemeMapConfig {
this.padding = {
top: () => window.innerWidth <= this.mobileBreakpointMax ? 150 : 50,
bottom: () => 50,
right: () => 50,
left: () => this.getLeftVisibleBoundary(),
right: () => isRTL(this.language) ? this.getVisibleBoundary() : 50,
left: () => !isRTL(this.language) ? this.getVisibleBoundary() : 50
};

/**
Expand Down Expand Up @@ -238,11 +239,14 @@ export default class ThemeMapConfig {
}

/**
* Get the leftmost point on the map, such that pins will still be visible
* Get the leftmost point on the map, or rightmost point for RTL locales, such that pins
* will still be visible.
*
* @return {Number} The boundary (in pixels) for the visible area of the map, from the left
* hand side of the viewport
* or right hand side of the viewport depending on if the language displayed
* is left-to-right or right-to-left
*/
getLeftVisibleBoundary () {
getVisibleBoundary () {
if (window.innerWidth <= this.mobileBreakpointMax) {
return 50;
}
Expand Down
2 changes: 1 addition & 1 deletion templates/vertical-full-page-map/markup/map.hbs
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<div id="js-answersMap" class="Answers-map js-answersMap"></div>
<div id="js-answersMap" class="Answers-map js-answersMap" dir="ltr"></div>
2 changes: 1 addition & 1 deletion templates/vertical-map/markup/map.hbs
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<div class="Answers-map js-answersMap" id="js-answersMap"></div>
<div class="Answers-map js-answersMap" id="js-answersMap" dir="ltr"></div>

0 comments on commit 75d30ee

Please sign in to comment.