From ea49f6ba570ff66a9d7fdf62065a9b7ee27c598b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Habinshuti?= Date: Tue, 12 Mar 2019 15:40:45 +0300 Subject: [PATCH 01/15] Remove geolocation field in metadata form if coordinates are empty --- app/react/Forms/components/Geolocation.js | 10 +++++++++- .../components/specs/Geolocation.spec.js | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/app/react/Forms/components/Geolocation.js b/app/react/Forms/components/Geolocation.js index 720780a0e9..131e477a03 100644 --- a/app/react/Forms/components/Geolocation.js +++ b/app/react/Forms/components/Geolocation.js @@ -2,6 +2,10 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; import Map from 'app/Map/Map'; +function isCoordValid(coord) { + return typeof coord === 'number' && !isNaN(coord); +} + export default class Geolocation extends Component { constructor(props) { super(props); @@ -11,7 +15,11 @@ export default class Geolocation extends Component { } onChange(value) { - this.props.onChange(value); + if (!isCoordValid(value.lat) && !isCoordValid(value.lon)) { + this.props.onChange(); + } else { + this.props.onChange(value); + } } latChange(e) { diff --git a/app/react/Forms/components/specs/Geolocation.spec.js b/app/react/Forms/components/specs/Geolocation.spec.js index 9d504baadc..8b6eefb184 100644 --- a/app/react/Forms/components/specs/Geolocation.spec.js +++ b/app/react/Forms/components/specs/Geolocation.spec.js @@ -35,6 +35,16 @@ describe('Geolocation', () => { latInput.simulate('change', { target: { value: '19' } }); expect(props.onChange).toHaveBeenCalledWith({ lat: 19, lon: -17.2 }); }); + describe('if lon is empty and lat is set to empty', () => { + it('should call onChange without a value', () => { + props.value.lon = ''; + render(); + const inputs = component.find('input'); + const latInput = inputs.first(); + latInput.simulate('change', { target: { value: '' } }); + expect(props.onChange.calls.argsFor(0)).toEqual([]); + }); + }); }); describe('when lon changes', () => { @@ -45,5 +55,15 @@ describe('Geolocation', () => { lonInput.simulate('change', { target: { value: '28' } }); expect(props.onChange).toHaveBeenCalledWith({ lat: 32.18, lon: 28 }); }); + describe('if lat is empty and lon is set to empty', () => { + it('should call onChange without a value', () => { + props.value.lat = ''; + render(); + const inputs = component.find('input'); + const lonInput = inputs.last(); + lonInput.simulate('change', { target: { value: '' } }); + expect(props.onChange.calls.argsFor(0)).toEqual([]); + }); + }); }); }); From d78f924ab705eb012bb4e3bb1c885c25ea2787d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Habinshuti?= Date: Tue, 12 Mar 2019 16:11:38 +0300 Subject: [PATCH 02/15] Add button to clear geolocation coordinates --- app/react/App/scss/elements/_controls.scss | 2 +- app/react/Forms/components/Geolocation.js | 16 +- .../components/specs/Geolocation.spec.js | 22 +++ .../__snapshots__/Geolocation.spec.js.snap | 137 ++++++++++++++++++ 4 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 app/react/Forms/components/specs/__snapshots__/Geolocation.spec.js.snap diff --git a/app/react/App/scss/elements/_controls.scss b/app/react/App/scss/elements/_controls.scss index afc5ae3bdc..927cb15d4c 100644 --- a/app/react/App/scss/elements/_controls.scss +++ b/app/react/App/scss/elements/_controls.scss @@ -228,7 +228,7 @@ input[type="checkbox"][disabled] ~ label { cursor: not-allowed; } -.icon-selector { +.icon-selector, .clear-field-button { button { background: none; border: none; diff --git a/app/react/Forms/components/Geolocation.js b/app/react/Forms/components/Geolocation.js index 131e477a03..8152d6ad73 100644 --- a/app/react/Forms/components/Geolocation.js +++ b/app/react/Forms/components/Geolocation.js @@ -1,6 +1,7 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; import Map from 'app/Map/Map'; +import { Translate } from 'app/I18N'; function isCoordValid(coord) { return typeof coord === 'number' && !isNaN(coord); @@ -12,6 +13,7 @@ export default class Geolocation extends Component { this.latChange = this.latChange.bind(this); this.lonChange = this.lonChange.bind(this); this.mapClick = this.mapClick.bind(this); + this.clearCoordinates = this.clearCoordinates.bind(this); } onChange(value) { @@ -36,6 +38,11 @@ export default class Geolocation extends Component { this.onChange({ lat: parseFloat(event.lngLat[1]), lon: parseFloat(event.lngLat[0]) }); } + clearCoordinates(e) { + e.preventDefault(); + this.props.onChange(); + } + render() { const markers = []; const { value } = this.props; @@ -47,7 +54,7 @@ export default class Geolocation extends Component { lon = value.lon; } - if (lat && lon) { + if (isCoordValid(lat) && isCoordValid(lon)) { markers.push({ latitude: parseFloat(value.lat), longitude: parseFloat(value.lon) }); } return ( @@ -63,6 +70,13 @@ export default class Geolocation extends Component { + { (isCoordValid(lat) || isCoordValid(lon)) && ( +
+ +
+ )} ); } diff --git a/app/react/Forms/components/specs/Geolocation.spec.js b/app/react/Forms/components/specs/Geolocation.spec.js index 8b6eefb184..2407a108cb 100644 --- a/app/react/Forms/components/specs/Geolocation.spec.js +++ b/app/react/Forms/components/specs/Geolocation.spec.js @@ -66,4 +66,26 @@ describe('Geolocation', () => { }); }); }); + + it('should render button to clear fields', () => { + render(); + expect(component).toMatchSnapshot(); + }); + + it('should hide clear fields button when lat and lon are empty', () => { + props.value = { lat: '', lon: '' }; + render(); + expect(component).toMatchSnapshot(); + }); + + describe('when clear fields button is clicked', () => { + it('should call onChange without a value', () => { + render(); + const event = { preventDefault: jasmine.createSpy('preventDefault') }; + const button = component.find('.clear-field-button button').first(); + button.simulate('click', event); + expect(props.onChange.calls.argsFor(0)).toEqual([]); + expect(event.preventDefault).toHaveBeenCalled(); + }); + }); }); diff --git a/app/react/Forms/components/specs/__snapshots__/Geolocation.spec.js.snap b/app/react/Forms/components/specs/__snapshots__/Geolocation.spec.js.snap new file mode 100644 index 0000000000..f1ed867052 --- /dev/null +++ b/app/react/Forms/components/specs/__snapshots__/Geolocation.spec.js.snap @@ -0,0 +1,137 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Geolocation should hide clear fields button when lat and lon are empty 1`] = ` +
+ +
+
+ + +
+
+ + +
+
+
+`; + +exports[`Geolocation should render button to clear fields 1`] = ` +
+ +
+
+ + +
+
+ + +
+
+
+ +
+
+`; From 180c9533eda9d7d1141c3def688621c6d4c566e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Habinshuti?= Date: Tue, 12 Mar 2019 16:32:26 +0300 Subject: [PATCH 03/15] Fix linter error --- app/react/Forms/components/Geolocation.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/react/Forms/components/Geolocation.js b/app/react/Forms/components/Geolocation.js index 8152d6ad73..eef3739c55 100644 --- a/app/react/Forms/components/Geolocation.js +++ b/app/react/Forms/components/Geolocation.js @@ -4,6 +4,7 @@ import Map from 'app/Map/Map'; import { Translate } from 'app/I18N'; function isCoordValid(coord) { + // eslint-disable-next-line no-restricted-globals return typeof coord === 'number' && !isNaN(coord); } @@ -72,9 +73,9 @@ export default class Geolocation extends Component { { (isCoordValid(lat) || isCoordValid(lon)) && (
- +
)} From 83613c6da3fb5922b9d53ba7c572dd0116e9c6dc Mon Sep 17 00:00:00 2001 From: RafaPolit Date: Tue, 12 Mar 2019 14:17:48 -0500 Subject: [PATCH 04/15] Removed double export from migration. --- .../migrations/12-add-RTL-to-settings-languages/index.js | 8 +++----- .../specs/12-add-RTL-to-settings-languages.spec.js | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/api/migrations/migrations/12-add-RTL-to-settings-languages/index.js b/app/api/migrations/migrations/12-add-RTL-to-settings-languages/index.js index 81202f2fec..d0ee62fdc1 100644 --- a/app/api/migrations/migrations/12-add-RTL-to-settings-languages/index.js +++ b/app/api/migrations/migrations/12-add-RTL-to-settings-languages/index.js @@ -1,7 +1,3 @@ -export const rtlLanguagesList = [ - 'ar', 'dv', 'ha', 'he', 'ks', 'ku', 'ps', 'fa', 'ur', 'yi' -]; - export default { delta: 12, @@ -9,6 +5,8 @@ export default { description: 'Adds the missing RTL value for existing instances with RTL languages', + rtlLanguagesList: ['ar', 'dv', 'ha', 'he', 'ks', 'ku', 'ps', 'fa', 'ur', 'yi'], + async up(db) { process.stdout.write(`${this.name}...\r\n`); @@ -18,7 +16,7 @@ export default { languages = languages.map((l) => { const migratedLanguage = l; - if (rtlLanguagesList.includes(l.key)) { + if (this.rtlLanguagesList.includes(l.key)) { migratedLanguage.rtl = true; } diff --git a/app/api/migrations/migrations/12-add-RTL-to-settings-languages/specs/12-add-RTL-to-settings-languages.spec.js b/app/api/migrations/migrations/12-add-RTL-to-settings-languages/specs/12-add-RTL-to-settings-languages.spec.js index d512107b6d..07f9af7924 100644 --- a/app/api/migrations/migrations/12-add-RTL-to-settings-languages/specs/12-add-RTL-to-settings-languages.spec.js +++ b/app/api/migrations/migrations/12-add-RTL-to-settings-languages/specs/12-add-RTL-to-settings-languages.spec.js @@ -1,6 +1,6 @@ import { catchErrors } from 'api/utils/jasmineHelpers'; import testingDB from 'api/utils/testing_db'; -import migration, { rtlLanguagesList } from '../index.js'; +import migration from '../index.js'; import fixtures from './fixtures.js'; describe('migration add-RTL-to-settings-languages', () => { @@ -27,7 +27,7 @@ describe('migration add-RTL-to-settings-languages', () => { expect(rtlLanguages.length).toBe(10); rtlLanguages.forEach((language) => { - expect(rtlLanguagesList).toContain(language.key); + expect(migration.rtlLanguagesList).toContain(language.key); }); }); From 2672db5f442c5f4412b15595908cee1f1526da8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Habinshuti?= Date: Wed, 13 Mar 2019 11:30:38 +0300 Subject: [PATCH 05/15] Add type property to Geolocation button --- app/react/Forms/components/Geolocation.js | 10 ++++------ app/react/Forms/components/specs/Geolocation.spec.js | 4 +--- .../specs/__snapshots__/Geolocation.spec.js.snap | 1 + 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/app/react/Forms/components/Geolocation.js b/app/react/Forms/components/Geolocation.js index eef3739c55..c0d438a700 100644 --- a/app/react/Forms/components/Geolocation.js +++ b/app/react/Forms/components/Geolocation.js @@ -19,10 +19,9 @@ export default class Geolocation extends Component { onChange(value) { if (!isCoordValid(value.lat) && !isCoordValid(value.lon)) { - this.props.onChange(); - } else { - this.props.onChange(value); + return this.props.onChange(); } + return this.props.onChange(value); } latChange(e) { @@ -39,8 +38,7 @@ export default class Geolocation extends Component { this.onChange({ lat: parseFloat(event.lngLat[1]), lon: parseFloat(event.lngLat[0]) }); } - clearCoordinates(e) { - e.preventDefault(); + clearCoordinates() { this.props.onChange(); } @@ -73,7 +71,7 @@ export default class Geolocation extends Component { { (isCoordValid(lat) || isCoordValid(lon)) && (
-
diff --git a/app/react/Forms/components/specs/Geolocation.spec.js b/app/react/Forms/components/specs/Geolocation.spec.js index 2407a108cb..dabe7647c9 100644 --- a/app/react/Forms/components/specs/Geolocation.spec.js +++ b/app/react/Forms/components/specs/Geolocation.spec.js @@ -81,11 +81,9 @@ describe('Geolocation', () => { describe('when clear fields button is clicked', () => { it('should call onChange without a value', () => { render(); - const event = { preventDefault: jasmine.createSpy('preventDefault') }; const button = component.find('.clear-field-button button').first(); - button.simulate('click', event); + button.simulate('click'); expect(props.onChange.calls.argsFor(0)).toEqual([]); - expect(event.preventDefault).toHaveBeenCalled(); }); }); }); diff --git a/app/react/Forms/components/specs/__snapshots__/Geolocation.spec.js.snap b/app/react/Forms/components/specs/__snapshots__/Geolocation.spec.js.snap index f1ed867052..47ff76c1e7 100644 --- a/app/react/Forms/components/specs/__snapshots__/Geolocation.spec.js.snap +++ b/app/react/Forms/components/specs/__snapshots__/Geolocation.spec.js.snap @@ -127,6 +127,7 @@ exports[`Geolocation should render button to clear fields 1`] = ` >