From 60936bae48225849c4519f87c6d987d9ed6f33ec Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Thu, 6 May 2021 16:11:17 +0200 Subject: [PATCH 1/4] #6830. Disable URL update on initial scroll --- web/client/epics/geostory.js | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/web/client/epics/geostory.js b/web/client/epics/geostory.js index dbf0505548..3adcee5fed 100644 --- a/web/client/epics/geostory.js +++ b/web/client/epics/geostory.js @@ -17,6 +17,8 @@ import words from 'lodash/words'; import get from 'lodash/get'; import isArray from 'lodash/isArray'; import isEmpty from 'lodash/isEmpty'; +import groupBy from "lodash/groupBy"; + import { push, LOCATION_CHANGE } from 'connected-react-router'; import uuid from 'uuid/v1'; @@ -503,6 +505,12 @@ export const handlePendingGeoStoryChanges = action$ => ) ); +const semaphore = (sem$, start = true, condition = (c) => c) => (stream$) => + stream$ + .withLatestFrom(sem$.startWith(start)) + .filter(([, s]) => condition(s)) + .map(([e]) => e); + /** * Handle the url updates on currentPage change * @param {Observable} action$ stream of actions @@ -510,6 +518,10 @@ export const handlePendingGeoStoryChanges = action$ => */ export const urlUpdateOnScroll = (action$, {getState}) => action$.ofType(UPDATE_CURRENT_PAGE) + .let(semaphore( + action$.ofType("GEOSTORY:SCROLLING") + .map(a => !a.value) + )) .debounceTime(50) // little delay if too many UPDATE_CURRENT_PAGE actions come .switchMap(({sectionId, columnId}) => { if ( @@ -533,14 +545,21 @@ export const scrollOnLoad = (action$) => action$.ofType(SET_CURRENT_STORY) .switchMap(() => { const storyIds = window?.location?.hash?.split('/'); - if (window?.location?.hash?.includes('shared')) { - scrollToContent(storyIds[7] || storyIds[5], {block: "start", behavior: "auto"}); - } else if (storyIds.length > 5) { - scrollToContent(storyIds[6], {block: "start", behavior: "auto"}); - } else if (storyIds.length === 5) { - scrollToContent(storyIds[4], {block: 'start', behavior: "auto"}); - } - return Observable.empty(); + return Observable.of(storyIds) + .delay(500) + .do(() => { + if (window?.location?.hash?.includes('shared')) { + scrollToContent(storyIds[7] || storyIds[5], {block: "start", behavior: "auto"}); + } else if (storyIds.length > 5) { + scrollToContent(storyIds[6], {block: "start", behavior: "auto"}); + } else if (storyIds.length === 5) { + scrollToContent(storyIds[4], {block: 'start', behavior: "auto"}); + } + } + ) + .ignoreElements() + .startWith({type: "GEOSTORY:SCROLLING", value: true}) + .concat(Observable.of({type: "GEOSTORY:SCROLLING", value: false}).delay(500)); }); /** From ba3f3df7704aa5d6f93373f06e5e2373c2a7a618 Mon Sep 17 00:00:00 2001 From: Matteo V Date: Tue, 11 May 2021 09:50:09 +0200 Subject: [PATCH 2/4] add test and externalize actions (#31) --- web/client/actions/__tests__/geostory-test.js | 9 +++++- web/client/actions/geostory.js | 3 ++ web/client/epics/__tests__/geostory-test.js | 29 +++++++++++++++++-- web/client/epics/geostory.js | 11 +++---- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/web/client/actions/__tests__/geostory-test.js b/web/client/actions/__tests__/geostory-test.js index 1028bb99d0..4240315418 100644 --- a/web/client/actions/__tests__/geostory-test.js +++ b/web/client/actions/__tests__/geostory-test.js @@ -51,10 +51,17 @@ import { updateSetting, removeResource, REMOVE_RESOURCE, updateUrlOnScroll, SET_UPDATE_URL_SCROLL, - updateMediaEditorSettings + updateMediaEditorSettings, + geostoryScrolling, + GEOSTORY_SCROLLING } from '../geostory'; describe('test geostory action creators', () => { + it('geostoryScrolling', () => { + const status = true; + const action = geostoryScrolling(status); + expect(action).toEqual({type: GEOSTORY_SCROLLING, status}); + }); it('clearSaveError', () => { const action = clearSaveError(); expect(action.type).toBe(CLEAR_SAVE_ERROR); diff --git a/web/client/actions/geostory.js b/web/client/actions/geostory.js index a1672a7020..55ca8cf4bc 100644 --- a/web/client/actions/geostory.js +++ b/web/client/actions/geostory.js @@ -17,6 +17,7 @@ export const EDIT_RESOURCE = "GEOSTORY:EDIT_RESOURCE"; export const EDIT_WEBPAGE = "GEOSTORY:EDIT_WEBPAGE"; export const ERRORS_LOGO = "GEOSTORY:ERRORS_LOGO"; export const GEOSTORY_LOADED = "GEOSTORY:GEOSTORY_LOADED"; +export const GEOSTORY_SCROLLING = "GEOSTORY:SCROLLING"; export const LOAD_GEOSTORY = "GEOSTORY:LOAD_GEOSTORY"; export const LOAD_GEOSTORY_ERROR = "GEOSTORY:LOAD_GEOSTORY_ERROR"; export const LOADING_GEOSTORY = "GEOSTORY:LOADING_GEOSTORY"; @@ -241,3 +242,5 @@ export const setPendingChanges = value => ({type: SET_PENDING_CHANGES, value}); export const updateUrlOnScroll = value => ({type: SET_UPDATE_URL_SCROLL, value}); export const updateMediaEditorSettings = mediaEditorSettings => ({ type: UPDATE_MEDIA_EDITOR_SETTINGS, mediaEditorSettings }); + +export const geostoryScrolling = (status) => ({ type: GEOSTORY_SCROLLING, status}); diff --git a/web/client/epics/__tests__/geostory-test.js b/web/client/epics/__tests__/geostory-test.js index 6602f89090..ad1b31b75f 100644 --- a/web/client/epics/__tests__/geostory-test.js +++ b/web/client/epics/__tests__/geostory-test.js @@ -32,13 +32,17 @@ import { openWebPageComponentCreator, editWebPageComponent, handlePendingGeoStoryChanges, - loadStoryOnHistoryPop + loadStoryOnHistoryPop, + scrollOnLoad, + urlUpdateOnScroll } from '../geostory'; import { ADD, LOADING_GEOSTORY, loadGeostory, SET_CURRENT_STORY, + setCurrentStory, + GEOSTORY_SCROLLING, LOAD_GEOSTORY_ERROR, add, UPDATE, @@ -57,7 +61,9 @@ import { editWebPage, setResource, SET_PENDING_CHANGES, - LOAD_GEOSTORY + LOAD_GEOSTORY, + geostoryScrolling, + updateCurrentPage } from '../../actions/geostory'; import { SET_CONTROL_PROPERTY } from '../../actions/controls'; import { @@ -1630,6 +1636,25 @@ describe('Geostory Epics', () => { }); }); + it('urlUpdateOnScroll', (done) => { + const NUM_ACTIONS = 1; + testEpic(addTimeoutEpic(urlUpdateOnScroll, 1000), NUM_ACTIONS, [updateCurrentPage({sectionId: "sectionId"})], + (actions) => { + expect(actions[0].type).toBe(TEST_TIMEOUT); + done(); + }, {geostory: {mode: "view", updateUrlOnScroll: true, resource: {id: "1"}}}); + }); + it('scrollOnLoad', (done) => { + const NUM_ACTIONS = 2; + testEpic(scrollOnLoad, NUM_ACTIONS, setCurrentStory({}), + (actions) => { + expect(actions[0].type).toBe(GEOSTORY_SCROLLING); + expect(actions[0].status).toBe(true); + expect(actions[1].type).toBe(GEOSTORY_SCROLLING); + expect(actions[1].status).toBe(false); + done(); + }); + }); describe('loadStoryOnHistoryPop', () => { it('loadStoryOnHistoryPop without shared', (done) => { const NUM_ACTIONS = 1; diff --git a/web/client/epics/geostory.js b/web/client/epics/geostory.js index 3adcee5fed..8fd5e77e48 100644 --- a/web/client/epics/geostory.js +++ b/web/client/epics/geostory.js @@ -17,7 +17,6 @@ import words from 'lodash/words'; import get from 'lodash/get'; import isArray from 'lodash/isArray'; import isEmpty from 'lodash/isEmpty'; -import groupBy from "lodash/groupBy"; import { push, LOCATION_CHANGE } from 'connected-react-router'; import uuid from 'uuid/v1'; @@ -58,7 +57,9 @@ import { EDIT_RESOURCE, UPDATE_CURRENT_PAGE, UPDATE_SETTING, - SET_CURRENT_STORY + SET_CURRENT_STORY, + geostoryScrolling, + GEOSTORY_SCROLLING } from '../actions/geostory'; import { setControlProperty } from '../actions/controls'; @@ -519,7 +520,7 @@ const semaphore = (sem$, start = true, condition = (c) => c) => (stream$) => export const urlUpdateOnScroll = (action$, {getState}) => action$.ofType(UPDATE_CURRENT_PAGE) .let(semaphore( - action$.ofType("GEOSTORY:SCROLLING") + action$.ofType(GEOSTORY_SCROLLING) .map(a => !a.value) )) .debounceTime(50) // little delay if too many UPDATE_CURRENT_PAGE actions come @@ -558,8 +559,8 @@ export const scrollOnLoad = (action$) => } ) .ignoreElements() - .startWith({type: "GEOSTORY:SCROLLING", value: true}) - .concat(Observable.of({type: "GEOSTORY:SCROLLING", value: false}).delay(500)); + .startWith(geostoryScrolling(true)) + .concat(Observable.of(geostoryScrolling(false)).delay(500)); }); /** From db5fc0679f225e1aafd3cdae323263b20f7e36fa Mon Sep 17 00:00:00 2001 From: Matteo V Date: Tue, 11 May 2021 15:46:09 +0200 Subject: [PATCH 3/4] parametrize delay (#32) --- web/client/epics/__tests__/geostory-test.js | 2 +- web/client/epics/geostory.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/web/client/epics/__tests__/geostory-test.js b/web/client/epics/__tests__/geostory-test.js index ad1b31b75f..2e5722cbb2 100644 --- a/web/client/epics/__tests__/geostory-test.js +++ b/web/client/epics/__tests__/geostory-test.js @@ -1638,7 +1638,7 @@ describe('Geostory Epics', () => { it('urlUpdateOnScroll', (done) => { const NUM_ACTIONS = 1; - testEpic(addTimeoutEpic(urlUpdateOnScroll, 1000), NUM_ACTIONS, [updateCurrentPage({sectionId: "sectionId"})], + testEpic(addTimeoutEpic(urlUpdateOnScroll, 100), NUM_ACTIONS, [updateCurrentPage({sectionId: "sectionId"})], (actions) => { expect(actions[0].type).toBe(TEST_TIMEOUT); done(); diff --git a/web/client/epics/geostory.js b/web/client/epics/geostory.js index 8fd5e77e48..e0a06670cf 100644 --- a/web/client/epics/geostory.js +++ b/web/client/epics/geostory.js @@ -544,10 +544,10 @@ export const urlUpdateOnScroll = (action$, {getState}) => */ export const scrollOnLoad = (action$) => action$.ofType(SET_CURRENT_STORY) - .switchMap(() => { + .switchMap(({delay = 500}) => { const storyIds = window?.location?.hash?.split('/'); return Observable.of(storyIds) - .delay(500) + .delay(delay) .do(() => { if (window?.location?.hash?.includes('shared')) { scrollToContent(storyIds[7] || storyIds[5], {block: "start", behavior: "auto"}); @@ -560,7 +560,7 @@ export const scrollOnLoad = (action$) => ) .ignoreElements() .startWith(geostoryScrolling(true)) - .concat(Observable.of(geostoryScrolling(false)).delay(500)); + .concat(Observable.of(geostoryScrolling(false)).delay(delay)); }); /** From 6042ad50f2bd26a409cc4ad5882f7d0e91e334c6 Mon Sep 17 00:00:00 2001 From: Lorenzo Natali Date: Tue, 11 May 2021 15:49:20 +0200 Subject: [PATCH 4/4] Fixed lint --- web/client/epics/__tests__/geostory-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/web/client/epics/__tests__/geostory-test.js b/web/client/epics/__tests__/geostory-test.js index 2e5722cbb2..df9a2531f0 100644 --- a/web/client/epics/__tests__/geostory-test.js +++ b/web/client/epics/__tests__/geostory-test.js @@ -62,7 +62,6 @@ import { setResource, SET_PENDING_CHANGES, LOAD_GEOSTORY, - geostoryScrolling, updateCurrentPage } from '../../actions/geostory'; import { SET_CONTROL_PROPERTY } from '../../actions/controls';