From cd9f747b7a57409f5ef53e9dcccc5e914371ddab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pau=20Ferrer=20Oca=C3=B1a?= Date: Thu, 23 Aug 2018 13:28:30 +0200 Subject: [PATCH] MOBILE-2558 utils: Check scroll is not destroyed before perform --- src/addon/calendar/pages/list/list.ts | 3 +- .../messages/pages/discussion/discussion.ts | 13 ++-- src/addon/mod/book/components/index/index.ts | 2 +- src/addon/mod/chat/pages/chat/chat.ts | 8 +- .../mod/choice/components/index/index.ts | 6 +- src/addon/mod/data/components/index/index.ts | 2 +- src/addon/mod/data/pages/edit/edit.ts | 2 +- src/addon/mod/data/pages/entry/entry.ts | 2 +- .../mod/feedback/components/index/index.ts | 2 +- src/addon/mod/feedback/pages/form/form.ts | 2 +- .../mod/forum/pages/discussion/discussion.ts | 4 +- .../mod/glossary/components/index/index.ts | 2 +- src/addon/mod/quiz/components/index/index.ts | 4 +- src/addon/mod/quiz/pages/player/player.ts | 6 +- src/addon/mod/quiz/pages/review/review.ts | 2 +- .../mod/workshop/components/index/index.ts | 2 +- .../workshop/pages/submission/submission.ts | 2 +- src/addon/notes/components/list/list.ts | 2 +- .../rich-text-editor/rich-text-editor.ts | 4 +- .../course/classes/main-activity-component.ts | 4 +- src/core/course/components/module/module.scss | 3 +- src/core/course/pages/section/section.ts | 2 +- src/providers/utils/dom.ts | 75 ++++++++++++++++++- 23 files changed, 114 insertions(+), 40 deletions(-) diff --git a/src/addon/calendar/pages/list/list.ts b/src/addon/calendar/pages/list/list.ts index 4fe69c33e..324d4b76f 100644 --- a/src/addon/calendar/pages/list/list.ts +++ b/src/addon/calendar/pages/list/list.ts @@ -306,7 +306,8 @@ export class AddonCalendarListPage implements OnDestroy { popover.onDidDismiss((course) => { if (course) { this.filter.course = course; - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); + this.filteredEvents = this.getFilteredEvents(); } }); diff --git a/src/addon/messages/pages/discussion/discussion.ts b/src/addon/messages/pages/discussion/discussion.ts index 20401a17a..e8cb226dc 100644 --- a/src/addon/messages/pages/discussion/discussion.ts +++ b/src/addon/messages/pages/discussion/discussion.ts @@ -232,7 +232,8 @@ export class AddonMessagesDiscussionPage implements OnDestroy { } // Check if we are at the bottom to scroll it after render. - this.scrollBottom = this.content.scrollHeight - this.content.scrollTop === this.content.contentHeight; + this.scrollBottom = this.domUtils.getScrollHeight(this.content) - this.domUtils.getScrollTop(this.content) === + this.domUtils.getContentHeight(this.content); if (this.messagesBeingSent > 0) { // Ignore polling due to a race condition. @@ -569,15 +570,15 @@ export class AddonMessagesDiscussionPage implements OnDestroy { // Wait for new content height to be calculated. setTimeout(() => { // Visible content size changed, maintain the bottom position. - if (!this.viewDestroyed && this.content && this.content.contentHeight != this.oldContentHeight) { + if (!this.viewDestroyed && this.content && this.domUtils.getContentHeight(this.content) != this.oldContentHeight) { if (!top) { top = this.content.getContentDimensions().scrollTop; } - top += this.oldContentHeight - this.content.contentHeight; - this.oldContentHeight = this.content.contentHeight; + top += this.oldContentHeight - this.domUtils.getContentHeight(this.content); + this.oldContentHeight = this.domUtils.getContentHeight(this.content); - this.content.scrollTo(0, top, 0); + this.domUtils.scrollTo(this.content, 0, top, 0); } }); } @@ -591,7 +592,7 @@ export class AddonMessagesDiscussionPage implements OnDestroy { // Need a timeout to leave time to the view to be rendered. setTimeout(() => { if (!this.viewDestroyed) { - this.content.scrollToBottom(0); + this.domUtils.scrollToBottom(this.content, 0); } }); this.scrollBottom = false; diff --git a/src/addon/mod/book/components/index/index.ts b/src/addon/mod/book/components/index/index.ts index e419d9716..4db347675 100644 --- a/src/addon/mod/book/components/index/index.ts +++ b/src/addon/mod/book/components/index/index.ts @@ -155,7 +155,7 @@ export class AddonModBookIndexComponent extends CoreCourseModuleMainResourceComp */ protected loadChapter(chapterId: string): Promise { this.currentChapter = chapterId; - this.content && this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return this.bookProvider.getChapterContent(this.contentsMap, chapterId, this.module.id).then((content) => { this.chapterContent = content; diff --git a/src/addon/mod/chat/pages/chat/chat.ts b/src/addon/mod/chat/pages/chat/chat.ts index 3898b67e1..99bcbec94 100644 --- a/src/addon/mod/chat/pages/chat/chat.ts +++ b/src/addon/mod/chat/pages/chat/chat.ts @@ -288,7 +288,7 @@ export class AddonModChatChatPage { // Need a timeout to leave time to the view to be rendered. setTimeout(() => { if (!this.viewDestroyed) { - this.content.scrollToBottom(0); + this.domUtils.scrollToBottom(this.content, 0); } }); } @@ -303,13 +303,13 @@ export class AddonModChatChatPage { // Wait for new content height to be calculated. setTimeout(() => { // Visible content size changed, maintain the bottom position. - if (!this.viewDestroyed && this.content && this.content.contentHeight != this.oldContentHeight) { + if (!this.viewDestroyed && this.content && this.domUtils.getContentHeight(this.content) != this.oldContentHeight) { if (!top) { top = this.content.getContentDimensions().scrollTop; } - top += this.oldContentHeight - this.content.contentHeight; - this.oldContentHeight = this.content.contentHeight; + top += this.oldContentHeight - this.domUtils.getContentHeight(this.content); + this.oldContentHeight = this.domUtils.getContentHeight(this.content); this.content.scrollTo(0, top, 0); } diff --git a/src/addon/mod/choice/components/index/index.ts b/src/addon/mod/choice/components/index/index.ts index 1465be6ff..ae899d407 100644 --- a/src/addon/mod/choice/components/index/index.ts +++ b/src/addon/mod/choice/components/index/index.ts @@ -100,7 +100,7 @@ export class AddonModChoiceIndexComponent extends CoreCourseModuleMainActivityCo */ protected isRefreshSyncNeeded(syncEventData: any): boolean { if (this.choice && syncEventData.choiceId == this.choice.id && syncEventData.userId == this.userId) { - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return true; } @@ -355,7 +355,7 @@ export class AddonModChoiceIndexComponent extends CoreCourseModuleMainActivityCo // Success! // Check completion since it could be configured to complete once the user answers the choice. this.courseProvider.checkModuleCompletion(this.courseId, this.module.completionstatus); - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); // Let's refresh the data. return this.refreshContent(false); @@ -374,7 +374,7 @@ export class AddonModChoiceIndexComponent extends CoreCourseModuleMainActivityCo this.domUtils.showConfirm(this.translate.instant('core.areyousure')).then(() => { const modal = this.domUtils.showModalLoading('core.sending', true); this.choiceProvider.deleteResponses(this.choice.id, this.choice.name, this.courseId).then(() => { - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); // Success! Let's refresh the data. return this.refreshContent(false); diff --git a/src/addon/mod/data/components/index/index.ts b/src/addon/mod/data/components/index/index.ts index 8f9e00fa0..f5007ca5a 100644 --- a/src/addon/mod/data/components/index/index.ts +++ b/src/addon/mod/data/components/index/index.ts @@ -142,7 +142,7 @@ export class AddonModDataIndexComponent extends CoreCourseModuleMainActivityComp if (this.data && syncEventData.dataId == this.data.id && typeof syncEventData.entryId == 'undefined') { this.loaded = false; // Refresh the data. - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return true; } diff --git a/src/addon/mod/data/pages/edit/edit.ts b/src/addon/mod/data/pages/edit/edit.ts index 3220a679d..270caa0dc 100644 --- a/src/addon/mod/data/pages/edit/edit.ts +++ b/src/addon/mod/data/pages/edit/edit.ts @@ -359,7 +359,7 @@ export class AddonModDataEditPage { */ protected scrollToFirstError(): void { if (!this.domUtils.scrollToElementBySelector(this.content, '.addon-data-error')) { - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); } } } diff --git a/src/addon/mod/data/pages/entry/entry.ts b/src/addon/mod/data/pages/entry/entry.ts index b43816471..815611c97 100644 --- a/src/addon/mod/data/pages/entry/entry.ts +++ b/src/addon/mod/data/pages/entry/entry.ts @@ -202,7 +202,7 @@ export class AddonModDataEntryPage implements OnDestroy { this.domUtils.showErrorModalDefault(message, 'core.course.errorgetmodule', true); }).finally(() => { - this.content && this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); this.entryLoaded = true; }); } diff --git a/src/addon/mod/feedback/components/index/index.ts b/src/addon/mod/feedback/components/index/index.ts index 140f5a376..cc0e922d9 100644 --- a/src/addon/mod/feedback/components/index/index.ts +++ b/src/addon/mod/feedback/components/index/index.ts @@ -135,7 +135,7 @@ export class AddonModFeedbackIndexComponent extends CoreCourseModuleMainActivity protected isRefreshSyncNeeded(syncEventData: any): boolean { if (this.feedback && syncEventData.feedbackId == this.feedback.id) { // Refresh the data. - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return true; } diff --git a/src/addon/mod/feedback/pages/form/form.ts b/src/addon/mod/feedback/pages/form/form.ts index fc1677027..c4329475b 100644 --- a/src/addon/mod/feedback/pages/form/form.ts +++ b/src/addon/mod/feedback/pages/form/form.ts @@ -247,7 +247,7 @@ export class AddonModFeedbackFormPage implements OnDestroy { * @return {Promise} Resolved when done. */ gotoPage(goPrevious: boolean): Promise { - this.content && this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); this.feedbackLoaded = false; const responses = this.feedbackHelper.getPageItemsResponses(this.items), diff --git a/src/addon/mod/forum/pages/discussion/discussion.ts b/src/addon/mod/forum/pages/discussion/discussion.ts index a063e5367..e652dab7c 100644 --- a/src/addon/mod/forum/pages/discussion/discussion.ts +++ b/src/addon/mod/forum/pages/discussion/discussion.ts @@ -366,7 +366,7 @@ export class AddonModForumDiscussionPage implements OnDestroy { * @return {Promise} Promise resolved when done. */ refreshPosts(sync?: boolean, showErrors?: boolean): Promise { - this.content && this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); this.refreshIcon = 'spinner'; this.syncIcon = 'spinner'; @@ -386,7 +386,7 @@ export class AddonModForumDiscussionPage implements OnDestroy { changeSort(type: SortType): Promise { this.discussionLoaded = false; this.sort = type; - this.content && this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return this.fetchPosts(); } diff --git a/src/addon/mod/glossary/components/index/index.ts b/src/addon/mod/glossary/components/index/index.ts index 9198e0ac9..461178374 100644 --- a/src/addon/mod/glossary/components/index/index.ts +++ b/src/addon/mod/glossary/components/index/index.ts @@ -314,7 +314,7 @@ export class AddonModGlossaryIndexComponent extends CoreCourseModuleMainActivity } this.loadingMessage = this.translate.instant('core.loading'); - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); this.switchMode(newMode); if (this.fetchMode === 'search') { diff --git a/src/addon/mod/quiz/components/index/index.ts b/src/addon/mod/quiz/components/index/index.ts index 8b77bc82f..d0cb002a8 100644 --- a/src/addon/mod/quiz/components/index/index.ts +++ b/src/addon/mod/quiz/components/index/index.ts @@ -426,7 +426,7 @@ export class AddonModQuizIndexComponent extends CoreCourseModuleMainActivityComp this.loaded = false; this.refreshIcon = 'spinner'; this.syncIcon = 'spinner'; - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); promise.then(() => { this.refreshContent().finally(() => { @@ -488,7 +488,7 @@ export class AddonModQuizIndexComponent extends CoreCourseModuleMainActivityComp } if (this.quizData && syncEventData.quizId == this.quizData.id) { - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return true; } diff --git a/src/addon/mod/quiz/pages/player/player.ts b/src/addon/mod/quiz/pages/player/player.ts index e6c36a6df..611fc59e9 100644 --- a/src/addon/mod/quiz/pages/player/player.ts +++ b/src/addon/mod/quiz/pages/player/player.ts @@ -182,11 +182,11 @@ export class AddonModQuizPlayerPage implements OnInit, OnDestroy { scrollLeft = scrollElement.scrollLeft || 0; this.loaded = false; - this.content.scrollToTop(); // Scroll top so the spinner is seen. + this.domUtils.scrollToTop(this.content); // Scroll top so the spinner is seen. return this.loadPage(this.attempt.currentpage).finally(() => { this.loaded = true; - this.content.scrollTo(scrollLeft, scrollTop); + this.domUtils.scrollTo(this.content, scrollLeft, scrollTop); }); }).finally(() => { modal.dismiss(); @@ -222,7 +222,7 @@ export class AddonModQuizPlayerPage implements OnInit, OnDestroy { } this.loaded = false; - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); // First try to save the attempt data. We only save it if we're not seeing the summary. const promise = this.showSummary ? Promise.resolve() : this.processAttempt(false, false); diff --git a/src/addon/mod/quiz/pages/review/review.ts b/src/addon/mod/quiz/pages/review/review.ts index 05d9c9c16..805d9662b 100644 --- a/src/addon/mod/quiz/pages/review/review.ts +++ b/src/addon/mod/quiz/pages/review/review.ts @@ -104,7 +104,7 @@ export class AddonModQuizReviewPage implements OnInit { } this.loaded = false; - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); this.loadPage(page).catch((error) => { this.domUtils.showErrorModalDefault(error, 'addon.mod_quiz.errorgetquestions', true); diff --git a/src/addon/mod/workshop/components/index/index.ts b/src/addon/mod/workshop/components/index/index.ts index 4421edf87..97f3a0df3 100644 --- a/src/addon/mod/workshop/components/index/index.ts +++ b/src/addon/mod/workshop/components/index/index.ts @@ -165,7 +165,7 @@ export class AddonModWorkshopIndexComponent extends CoreCourseModuleMainActivity protected isRefreshSyncNeeded(syncEventData: any): boolean { if (this.workshop && syncEventData.workshopId == this.workshop.id) { // Refresh the data. - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return true; } diff --git a/src/addon/mod/workshop/pages/submission/submission.ts b/src/addon/mod/workshop/pages/submission/submission.ts index 6fba624d2..c9242e761 100644 --- a/src/addon/mod/workshop/pages/submission/submission.ts +++ b/src/addon/mod/workshop/pages/submission/submission.ts @@ -172,7 +172,7 @@ export class AddonModWorkshopSubmissionPage implements OnInit, OnDestroy { */ protected eventReceived(data: any): void { if (this.workshopId === data.workshopId) { - this.content && this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); this.loaded = false; this.refreshAllData(); diff --git a/src/addon/notes/components/list/list.ts b/src/addon/notes/components/list/list.ts index 65734650d..942670e11 100644 --- a/src/addon/notes/components/list/list.ts +++ b/src/addon/notes/components/list/list.ts @@ -56,7 +56,7 @@ export class AddonNotesListComponent implements OnInit, OnDestroy { this.refreshIcon = 'spinner'; this.syncIcon = 'spinner'; - this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); this.fetchNotes(false); } }, sitesProvider.getCurrentSiteId()); diff --git a/src/components/rich-text-editor/rich-text-editor.ts b/src/components/rich-text-editor/rich-text-editor.ts index d8111012b..1afb00682 100644 --- a/src/components/rich-text-editor/rich-text-editor.ts +++ b/src/components/rich-text-editor/rich-text-editor.ts @@ -141,7 +141,7 @@ export class CoreRichTextEditorComponent implements AfterContentInit, OnDestroy const deferred = this.utils.promiseDefer(); setTimeout(() => { - const contentVisibleHeight = this.content.contentHeight - this.kbHeight; + const contentVisibleHeight = this.domUtils.getContentHeight(this.content) - this.kbHeight; if (contentVisibleHeight <= 0) { deferred.resolve(0); @@ -164,7 +164,7 @@ export class CoreRichTextEditorComponent implements AfterContentInit, OnDestroy } } else { // Header is fixed, use the content to calculate the editor height. - height = this.content.contentHeight - this.kbHeight - this.getSurroundingHeight(this.element); + height = this.domUtils.getContentHeight(this.content) - this.kbHeight - this.getSurroundingHeight(this.element); } if (height > this.minHeight) { diff --git a/src/core/course/classes/main-activity-component.ts b/src/core/course/classes/main-activity-component.ts index d6b841dc4..16b6e8dd4 100644 --- a/src/core/course/classes/main-activity-component.ts +++ b/src/core/course/classes/main-activity-component.ts @@ -162,7 +162,7 @@ export class CoreCourseModuleMainActivityComponent extends CoreCourseModuleMainR this.refreshIcon = 'spinner'; this.syncIcon = 'spinner'; this.loaded = false; - this.content && this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return this.loadContent(false, sync, showErrors).finally(() => { this.refreshIcon = 'refresh'; @@ -181,7 +181,7 @@ export class CoreCourseModuleMainActivityComponent extends CoreCourseModuleMainR this.refreshIcon = 'spinner'; this.syncIcon = 'spinner'; this.loaded = false; - this.content && this.content.scrollToTop(); + this.domUtils.scrollToTop(this.content); return this.refreshContent(sync, showErrors); } diff --git a/src/core/course/components/module/module.scss b/src/core/course/components/module/module.scss index 9e1febfad..e71fa5721 100644 --- a/src/core/course/components/module/module.scss +++ b/src/core/course/components/module/module.scss @@ -27,7 +27,8 @@ core-course-module { core-format-text { flex-grow: 2; } - .core-module-buttons { + .core-module-buttons, + .buttons.core-module-buttons { margin: 0; } diff --git a/src/core/course/pages/section/section.ts b/src/core/course/pages/section/section.ts index 59b92d6f6..e2b51ce58 100644 --- a/src/core/course/pages/section/section.ts +++ b/src/core/course/pages/section/section.ts @@ -305,7 +305,7 @@ export class CoreCourseSectionPage implements OnDestroy { scrollLeft = scrollElement.scrollLeft || 0; this.dataLoaded = false; - this.content.scrollToTop(); // Scroll top so the spinner is seen. + this.domUtils.scrollToTop(this.content); // Scroll top so the spinner is seen. this.loadData().then(() => { return this.formatComponent.doRefresh(undefined, undefined, true); diff --git a/src/providers/utils/dom.ts b/src/providers/utils/dom.ts index 070f6892c..d8ebbe017 100644 --- a/src/providers/utils/dom.ts +++ b/src/providers/utils/dom.ts @@ -717,6 +717,77 @@ export class CoreDomUtilsProvider { return element.innerHTML; } + /** + * Scroll to somehere in the content. + * Checks hidden property _scroll to avoid errors if view is not active. + * + * @param {Content} content Content where to execute the function. + * @param {number} x The x-value to scroll to. + * @param {number} y The y-value to scroll to. + * @param {number} [duration] Duration of the scroll animation in milliseconds. Defaults to `300`. + * @returns {Promise} Returns a promise which is resolved when the scroll has completed. + */ + scrollTo(content: Content, x: number, y: number, duration?: number, done?: Function): Promise { + return content && content._scroll && content.scrollTo(x, y, duration, done); + } + + /** + * Scroll to Bottom of the content. + * Checks hidden property _scroll to avoid errors if view is not active. + * + * @param {Content} content Content where to execute the function. + * @param {number} [duration] Duration of the scroll animation in milliseconds. Defaults to `300`. + * @returns {Promise} Returns a promise which is resolved when the scroll has completed. + */ + scrollToBottom(content: Content, duration?: number): Promise { + return content && content._scroll && content.scrollToBottom(duration); + } + + /** + * Scroll to Top of the content. + * Checks hidden property _scroll to avoid errors if view is not active. + * + * @param {Content} content Content where to execute the function. + * @param {number} [duration] Duration of the scroll animation in milliseconds. Defaults to `300`. + * @returns {Promise} Returns a promise which is resolved when the scroll has completed. + */ + scrollToTop(content: Content, duration?: number): Promise { + return content && content._scroll && content.scrollToTop(duration); + } + + /** + * Returns contentHeight of the content. + * Checks hidden property _scroll to avoid errors if view is not active. + * + * @param {Content} content Content where to execute the function. + * @return {number} Content contentHeight or 0. + */ + getContentHeight(content: Content): number { + return (content && content._scroll && content.contentHeight) || 0; + } + + /** + * Returns scrollHeight of the content. + * Checks hidden property _scroll to avoid errors if view is not active. + * + * @param {Content} content Content where to execute the function. + * @return {number} Content scrollHeight or 0. + */ + getScrollHeight(content: Content): number { + return (content && content._scroll && content.scrollHeight) || 0; + } + + /** + * Returns scrollTop of the content. + * Checks hidden property _scroll to avoid errors if view is not active. + * + * @param {Content} content Content where to execute the function. + * @return {number} Content scrollTop or 0. + */ + getScrollTop(content: Content): number { + return (content && content._scroll && content.scrollTop) || 0; + } + /** * Scroll to a certain element. * @@ -731,7 +802,7 @@ export class CoreDomUtilsProvider { return false; } - content.scrollTo(position[0], position[1]); + this.scrollTo(content, position[0], position[1]); return true; } @@ -750,7 +821,7 @@ export class CoreDomUtilsProvider { return false; } - content.scrollTo(position[0], position[1]); + this.scrollTo(content, position[0], position[1]); return true; }