From 04ebb8e50ca3b67076c1d95e821f4ba80eeb3b9f Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Fri, 12 Apr 2019 14:37:45 +0100 Subject: [PATCH] MOBILE-2735 UX: Course and module links add new page to history. This ensures that "back" always goes back and "home" always goes home. --- src/addon/mod/book/providers/link-handler.ts | 2 +- .../lesson/providers/grade-link-handler.ts | 2 +- .../lesson/providers/index-link-handler.ts | 15 ++++++++----- .../classes/module-grade-handler.ts | 2 +- .../classes/module-index-handler.ts | 2 +- src/core/course/providers/helper.ts | 22 +++++++++++++++++-- .../courses/providers/course-link-handler.ts | 19 ++++++++++++---- 7 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/addon/mod/book/providers/link-handler.ts b/src/addon/mod/book/providers/link-handler.ts index 899978d4b..631885cbe 100644 --- a/src/addon/mod/book/providers/link-handler.ts +++ b/src/addon/mod/book/providers/link-handler.ts @@ -46,7 +46,7 @@ export class AddonModBookLinkHandler extends CoreContentLinksModuleIndexHandler return [{ action: (siteId, navCtrl?): void => { this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId, undefined, - this.useModNameToGetModule ? this.modName : undefined, modParams); + this.useModNameToGetModule ? this.modName : undefined, modParams, navCtrl); } }]; } diff --git a/src/addon/mod/lesson/providers/grade-link-handler.ts b/src/addon/mod/lesson/providers/grade-link-handler.ts index f71c50586..f76eb8b52 100644 --- a/src/addon/mod/lesson/providers/grade-link-handler.ts +++ b/src/addon/mod/lesson/providers/grade-link-handler.ts @@ -70,7 +70,7 @@ export class AddonModLessonGradeLinkHandler extends CoreContentLinksModuleGradeH this.linkHelper.goInSite(navCtrl, 'AddonModLessonUserRetakePage', pageParams, siteId); } else { // User cannot view the report, go to lesson index. - this.courseHelper.navigateToModule(moduleId, siteId, courseId, module.section); + this.courseHelper.navigateToModule(moduleId, siteId, courseId, module.section, undefined, undefined, navCtrl); } }).catch((error) => { this.domUtils.showErrorModalDefault(error, 'core.course.errorgetmodule', true); diff --git a/src/addon/mod/lesson/providers/index-link-handler.ts b/src/addon/mod/lesson/providers/index-link-handler.ts index 244c55fcd..16d77cd6e 100644 --- a/src/addon/mod/lesson/providers/index-link-handler.ts +++ b/src/addon/mod/lesson/providers/index-link-handler.ts @@ -19,6 +19,7 @@ import { CoreContentLinksAction } from '@core/contentlinks/providers/delegate'; import { CoreCourseProvider } from '@core/course/providers/course'; import { CoreCourseHelperProvider } from '@core/course/providers/helper'; import { AddonModLessonProvider } from './lesson'; +import { NavController } from 'ionic-angular'; /** * Handler to treat links to lesson index. @@ -51,9 +52,10 @@ export class AddonModLessonIndexLinkHandler extends CoreContentLinksModuleIndexH /* Ignore the pageid param. If we open the lesson player with a certain page and the user hasn't started the lesson, an error is thrown: could not find lesson_timer records. */ if (params.userpassword) { - this.navigateToModuleWithPassword(parseInt(params.id, 10), courseId, params.userpassword, siteId); + this.navigateToModuleWithPassword(parseInt(params.id, 10), courseId, params.userpassword, siteId, navCtrl); } else { - this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId); + this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId, + undefined, undefined, undefined, navCtrl); } } }]; @@ -80,9 +82,11 @@ export class AddonModLessonIndexLinkHandler extends CoreContentLinksModuleIndexH * @param {number} courseId Course ID. * @param {string} password Password. * @param {string} siteId Site ID. + * @param {NavController} navCtrl Navigation controller. * @return {Promise} Promise resolved when navigated. */ - protected navigateToModuleWithPassword(moduleId: number, courseId: number, password: string, siteId: string): Promise { + protected navigateToModuleWithPassword(moduleId: number, courseId: number, password: string, siteId: string, + navCtrl?: NavController): Promise { const modal = this.domUtils.showModalLoading(); // Get the module. @@ -93,11 +97,12 @@ export class AddonModLessonIndexLinkHandler extends CoreContentLinksModuleIndexH return this.lessonProvider.storePassword(parseInt(module.instance, 10), password, siteId).catch(() => { // Ignore errors. }).then(() => { - return this.courseHelper.navigateToModule(moduleId, siteId, courseId, module.section); + return this.courseHelper.navigateToModule(moduleId, siteId, courseId, module.section, + undefined, undefined, navCtrl); }); }).catch(() => { // Error, go to index page. - return this.courseHelper.navigateToModule(moduleId, siteId, courseId); + return this.courseHelper.navigateToModule(moduleId, siteId, courseId, undefined, undefined, undefined, navCtrl); }).finally(() => { modal.dismiss(); }); diff --git a/src/core/contentlinks/classes/module-grade-handler.ts b/src/core/contentlinks/classes/module-grade-handler.ts index a97b30a2a..c1a5124e4 100644 --- a/src/core/contentlinks/classes/module-grade-handler.ts +++ b/src/core/contentlinks/classes/module-grade-handler.ts @@ -77,7 +77,7 @@ export class CoreContentLinksModuleGradeHandler extends CoreContentLinksHandlerB if (!params.userid || params.userid == site.getUserId()) { // No user specified or current user. Navigate to module. this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId, undefined, - this.useModNameToGetModule ? this.modName : undefined); + this.useModNameToGetModule ? this.modName : undefined, undefined, navCtrl); } else if (this.canReview) { // Use the goToReview function. this.goToReview(url, params, courseId, siteId, navCtrl); diff --git a/src/core/contentlinks/classes/module-index-handler.ts b/src/core/contentlinks/classes/module-index-handler.ts index fbb65afba..67159b012 100644 --- a/src/core/contentlinks/classes/module-index-handler.ts +++ b/src/core/contentlinks/classes/module-index-handler.ts @@ -60,7 +60,7 @@ export class CoreContentLinksModuleIndexHandler extends CoreContentLinksHandlerB return [{ action: (siteId, navCtrl?): void => { this.courseHelper.navigateToModule(parseInt(params.id, 10), siteId, courseId, undefined, - this.useModNameToGetModule ? this.modName : undefined); + this.useModNameToGetModule ? this.modName : undefined, undefined, navCtrl); } }]; } diff --git a/src/core/course/providers/helper.ts b/src/core/course/providers/helper.ts index 0a7b95247..04a978423 100644 --- a/src/core/course/providers/helper.ts +++ b/src/core/course/providers/helper.ts @@ -36,6 +36,7 @@ import { CoreCourseModulePrefetchDelegate } from './module-prefetch-delegate'; import { CoreLoginHelperProvider } from '@core/login/providers/helper'; import { CoreConstants } from '@core/constants'; import { CoreSite } from '@classes/site'; +import { CoreLoggerProvider } from '@providers/logger'; import * as moment from 'moment'; /** @@ -115,6 +116,7 @@ export type CoreCourseCoursesProgress = { export class CoreCourseHelperProvider { protected courseDwnPromises: { [s: string]: { [id: number]: Promise } } = {}; + protected logger; constructor(private courseProvider: CoreCourseProvider, private domUtils: CoreDomUtilsProvider, private moduleDelegate: CoreCourseModuleDelegate, private prefetchDelegate: CoreCourseModulePrefetchDelegate, @@ -124,7 +126,10 @@ export class CoreCourseHelperProvider { private courseOptionsDelegate: CoreCourseOptionsDelegate, private siteHomeProvider: CoreSiteHomeProvider, private eventsProvider: CoreEventsProvider, private fileHelper: CoreFileHelperProvider, private appProvider: CoreAppProvider, private fileProvider: CoreFileProvider, private injector: Injector, - private coursesProvider: CoreCoursesProvider, private courseOffline: CoreCourseOfflineProvider) { } + private coursesProvider: CoreCoursesProvider, private courseOffline: CoreCourseOfflineProvider, + private loggerProvider: CoreLoggerProvider) { + this.logger = loggerProvider.getInstance('CoreCourseHelperProvider'); + } /** * This function treats every module on the sections provided to load the handler data, treat completion @@ -1109,9 +1114,12 @@ export class CoreCourseHelperProvider { * @param {string} [modName] If set, the app will retrieve all modules of this type with a single WS call. This reduces the * number of WS calls, but it isn't recommended for modules that can return a lot of contents. * @param {any} [modParams] Params to pass to the module + * @param {NavController} [navCtrl] NavController for adding new pages to the current history. Optional for legacy support, but + * generates a warning if omitted. * @return {Promise} Promise resolved when done. */ - navigateToModule(moduleId: number, siteId?: string, courseId?: number, sectionId?: number, modName?: string, modParams?: any) + navigateToModule(moduleId: number, siteId?: string, courseId?: number, sectionId?: number, modName?: string, modParams?: any, + navCtrl?: NavController) : Promise { siteId = siteId || this.sitesProvider.getCurrentSiteId(); @@ -1157,6 +1165,16 @@ export class CoreCourseHelperProvider { module.handlerData = this.moduleDelegate.getModuleDataFor(module.modname, module, courseId, sectionId); + if (navCtrl) { + // If the link handler for this module passed through navCtrl, we can use the module's handler to navigate cleanly. + // Otherwise, we will redirect below. + modal.dismiss(); + + return module.handlerData.action(event, navCtrl, module, courseId); + } + + this.logger.warn('navCtrl was not passed to navigateToModule by the link handler for ' + module.modname); + if (courseId == site.getSiteHomeId()) { // Check if site home is available. return this.siteHomeProvider.isAvailable().then(() => { diff --git a/src/core/courses/providers/course-link-handler.ts b/src/core/courses/providers/course-link-handler.ts index 010dcdbb2..dc688e1c4 100644 --- a/src/core/courses/providers/course-link-handler.ts +++ b/src/core/courses/providers/course-link-handler.ts @@ -22,6 +22,8 @@ import { CoreContentLinksAction } from '@core/contentlinks/providers/delegate'; import { CoreCourseProvider } from '@core/course/providers/course'; import { CoreCourseHelperProvider } from '@core/course/providers/helper'; import { CoreCoursesProvider } from './courses'; +import { NavController } from 'ionic-angular'; +import { CoreLoggerProvider } from '@providers/logger'; /** * Handler to treat links to course view or enrol (except site home). @@ -32,12 +34,15 @@ export class CoreCoursesCourseLinkHandler extends CoreContentLinksHandlerBase { pattern = /((\/enrol\/index\.php)|(\/course\/enrol\.php)|(\/course\/view\.php)).*([\?\&]id=\d+)/; protected waitStart = 0; + protected logger; constructor(private sitesProvider: CoreSitesProvider, private coursesProvider: CoreCoursesProvider, private domUtils: CoreDomUtilsProvider, private translate: TranslateService, private courseProvider: CoreCourseProvider, - private textUtils: CoreTextUtilsProvider, private courseHelper: CoreCourseHelperProvider) { + private textUtils: CoreTextUtilsProvider, private courseHelper: CoreCourseHelperProvider, + private loggerProvider: CoreLoggerProvider) { super(); + this.logger = loggerProvider.getInstance('CoreCoursesCourseLinkHandler'); } /** @@ -75,7 +80,7 @@ export class CoreCoursesCourseLinkHandler extends CoreContentLinksHandlerBase { action: (siteId, navCtrl?): void => { siteId = siteId || this.sitesProvider.getCurrentSiteId(); if (siteId == this.sitesProvider.getCurrentSiteId()) { - this.actionEnrol(courseId, url, pageParams).catch(() => { + this.actionEnrol(courseId, url, pageParams, navCtrl).catch(() => { // Ignore errors. }); } else { @@ -115,9 +120,11 @@ export class CoreCoursesCourseLinkHandler extends CoreContentLinksHandlerBase { * @param {number} courseId Course ID. * @param {string} url Treated URL. * @param {any} pageParams Params to send to the new page. + * @param {NavController} [navCtrl] NavController for adding new pages to the current history. Optional for legacy support, but + * generates a warning if omitted. * @return {Promise} Promise resolved when done. */ - protected actionEnrol(courseId: number, url: string, pageParams: any): Promise { + protected actionEnrol(courseId: number, url: string, pageParams: any, navCtrl?: NavController): Promise { const modal = this.domUtils.showModalLoading(), isEnrolUrl = !!url.match(/(\/enrol\/index\.php)|(\/course\/enrol\.php)/); let course; @@ -188,8 +195,12 @@ export class CoreCoursesCourseLinkHandler extends CoreContentLinksHandlerBase { }).then((course) => { modal.dismiss(); + if (typeof navCtrl === 'undefined') { + this.logger.warn('navCtrl was not passed to actionEnrol'); + } + // Now open the course. - this.courseHelper.openCourse(undefined, course, pageParams); + this.courseHelper.openCourse(navCtrl, course, pageParams); }); }