From 9770072b4450915035bf144a85ed47948a04c042 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Wed, 5 May 2021 11:01:16 +0200 Subject: [PATCH 1/3] MOBILE-3320 tabs: Fix issues with hiding tabs on scroll --- src/core/classes/tabs.ts | 46 +++++++++++-------- .../components/tabs-outlet/tabs-outlet.ts | 9 +++- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/core/classes/tabs.ts b/src/core/classes/tabs.ts index c04bea315..f62a34156 100644 --- a/src/core/classes/tabs.ts +++ b/src/core/classes/tabs.ts @@ -87,7 +87,7 @@ export class CoreTabsBaseComponent implements OnInit, Aft protected isInTransition = false; // Weather Slides is in transition. protected slidesSwiper: any; // eslint-disable-line @typescript-eslint/no-explicit-any protected slidesSwiperLoaded = false; - protected scrollListenersSet: Record = {}; // Prevent setting listeners twice. + protected scrollElements: Record = {}; // Scroll elements for each loaded tab. constructor( protected element: ElementRef, @@ -444,11 +444,11 @@ export class CoreTabsBaseComponent implements OnInit, Aft /** * Show or hide the tabs. This is used when the user is scrolling inside a tab. * - * @param scrollEvent Scroll event to check scroll position. - * @param content Content element to check measures. + * @param scrollTop Scroll top. + * @param scrollElement Content scroll element to check measures. */ - showHideTabs(scrollEvent: CustomEvent, content: HTMLElement): void { - if (!this.tabBarElement || !this.tabsElement || !content) { + showHideTabs(scrollTop: number, scrollElement: HTMLElement): void { + if (!this.tabBarElement || !this.tabsElement || !scrollElement) { return; } @@ -467,8 +467,7 @@ export class CoreTabsBaseComponent implements OnInit, Aft return; } - const scroll = parseInt(scrollEvent.detail.scrollTop, 10); - if (scroll <= 0) { + if (scrollTop <= 0) { // Ensure tabbar is shown. this.tabsElement.style.top = '0'; this.tabsElement.style.height = ''; @@ -479,30 +478,30 @@ export class CoreTabsBaseComponent implements OnInit, Aft return; } - if (scroll == this.lastScroll) { + if (scrollTop == this.lastScroll) { // Ensure scroll has been modified to avoid flicks. return; } - if (this.tabsShown && scroll > this.tabBarHeight) { + if (this.tabsShown && scrollTop > this.tabBarHeight) { this.tabsShown = false; // Hide tabs. this.tabBarElement.classList.add('tabs-hidden'); this.tabsElement.style.top = '0'; this.tabsElement.style.height = ''; - } else if (!this.tabsShown && scroll <= this.tabBarHeight) { + } else if (!this.tabsShown && scrollTop <= this.tabBarHeight) { this.tabsShown = true; this.tabBarElement.classList.remove('tabs-hidden'); } - if (this.tabsShown && content.scrollHeight > content.clientHeight + (this.tabBarHeight - scroll)) { + if (this.tabsShown && scrollElement.scrollHeight > scrollElement.clientHeight + (this.tabBarHeight - scrollTop)) { // Smooth translation. - this.tabsElement.style.top = - scroll + 'px'; - this.tabsElement.style.height = 'calc(100% + ' + scroll + 'px'; + this.tabsElement.style.top = - scrollTop + 'px'; + this.tabsElement.style.height = 'calc(100% + ' + scrollTop + 'px'; } // Use lastScroll after moving the tabs to avoid flickering. - this.lastScroll = parseInt(scrollEvent.detail.scrollTop, 10); + this.lastScroll = scrollTop; } /** @@ -580,17 +579,28 @@ export class CoreTabsBaseComponent implements OnInit, Aft * @return Promise resolved when done. */ async listenContentScroll(element: HTMLElement, id: number | string): Promise { - const content = element.querySelector('ion-content'); + if (this.scrollElements[id]) { + return; // Already set. + } - if (!content || this.scrollListenersSet[id]) { + let content = element.querySelector('ion-content'); + if (!content) { return; } + // Search the inner ion-content if there's more than one. + let childContent = content.querySelector('ion-content') || null; + while (childContent != null) { + content = childContent; + childContent = content.querySelector('ion-content') || null; + } + const scroll = await content.getScrollElement(); + content.scrollEvents = true; - this.scrollListenersSet[id] = true; + this.scrollElements[id] = scroll; content.addEventListener('ionScroll', (e: CustomEvent): void => { - this.showHideTabs(e, scroll); + this.showHideTabs(parseInt(e.detail.scrollTop, 10), scroll); }); } diff --git a/src/core/components/tabs-outlet/tabs-outlet.ts b/src/core/components/tabs-outlet/tabs-outlet.ts index 963cb3ced..cb256592b 100644 --- a/src/core/components/tabs-outlet/tabs-outlet.ts +++ b/src/core/components/tabs-outlet/tabs-outlet.ts @@ -99,8 +99,15 @@ export class CoreTabsOutletComponent extends CoreTabsBaseComponent Date: Wed, 5 May 2021 11:22:35 +0200 Subject: [PATCH 2/3] MOBILE-3320 tabs: Only slide to tab if not visible --- src/core/classes/tabs.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/classes/tabs.ts b/src/core/classes/tabs.ts index f62a34156..6338fc65d 100644 --- a/src/core/classes/tabs.ts +++ b/src/core/classes/tabs.ts @@ -545,7 +545,12 @@ export class CoreTabsBaseComponent implements OnInit, Aft } if (this.selected) { - await this.slides!.slideTo(index); + // Check if we need to slide to the tab because it's not visible. + const firstVisibleTab = await this.slides!.getActiveIndex(); + const lastVisibleTab = firstVisibleTab + this.slidesOpts.slidesPerView - 1; + if (index < firstVisibleTab || index > lastVisibleTab) { + await this.slides!.slideTo(index, 0, true); + } } const ok = await this.loadTab(tabToSelect); From d10e9f574abe7be130a9ef48964a612a5d0e448d Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Wed, 5 May 2021 12:24:57 +0200 Subject: [PATCH 3/3] MOBILE-3320 core: Fix grades link handler --- .../course/pages/index/index.module.ts | 3 + .../features/course/pages/index/index.page.ts | 11 +-- src/core/features/course/services/course.ts | 11 ++- .../features/grades/services/grades-helper.ts | 72 ++++++++----------- src/core/services/navigator.ts | 13 ++-- src/core/services/utils/utils.ts | 4 +- 6 files changed, 59 insertions(+), 55 deletions(-) diff --git a/src/core/features/course/pages/index/index.module.ts b/src/core/features/course/pages/index/index.module.ts index b8a598869..1e1d26938 100644 --- a/src/core/features/course/pages/index/index.module.ts +++ b/src/core/features/course/pages/index/index.module.ts @@ -27,6 +27,9 @@ function buildRoutes(injector: Injector): Routes { { path: '', component: CoreCourseIndexPage, + data: { + isCourseIndex: true, + }, children: routes.children, }, ...routes.siblings, diff --git a/src/core/features/course/pages/index/index.page.ts b/src/core/features/course/pages/index/index.page.ts index 6ba057186..461c3164f 100644 --- a/src/core/features/course/pages/index/index.page.ts +++ b/src/core/features/course/pages/index/index.page.ts @@ -71,7 +71,7 @@ export class CoreCourseIndexPage implements OnInit, OnDestroy { const index = this.tabs.findIndex((tab) => tab.name == data.name); if (index >= 0) { - this.tabsComponent?.selectByIndex(index + 1); + this.tabsComponent?.selectByIndex(index); } } }); @@ -129,11 +129,9 @@ export class CoreCourseIndexPage implements OnInit, OnDestroy { // Load the course handlers. const handlers = await CoreCourseOptionsDelegate.getHandlersToDisplay(this.course!, false, false); - this.tabs = [...this.tabs, ...handlers.map(handler => handler.data)]; - let tabToLoad: number | undefined; - // Add the courseId to the handler component data. + // Create the full path. handlers.forEach((handler, index) => { handler.data.page = CoreTextUtils.concatenatePaths(this.currentPagePath, handler.data.page); handler.data.pageParams = handler.data.pageParams || {}; @@ -144,6 +142,11 @@ export class CoreCourseIndexPage implements OnInit, OnDestroy { } }); + this.tabs = [...this.tabs, ...handlers.map(handler => ({ + ...handler.data, + name: handler.name, + }))]; + // Select the tab if needed. this.firstTabName = undefined; if (tabToLoad) { diff --git a/src/core/features/course/services/course.ts b/src/core/features/course/services/course.ts index 36e5ef902..4be90a405 100644 --- a/src/core/features/course/services/course.ts +++ b/src/core/features/course/services/course.ts @@ -43,6 +43,7 @@ import { CoreCourseLogCronHandler } from './handlers/log-cron'; import { CoreSitePlugins } from '@features/siteplugins/services/siteplugins'; import { CoreCourseAutoSyncData, CoreCourseSyncProvider } from './sync'; import { CoreTagItem } from '@features/tag/services/tag'; +import { CoreNavigator } from '@services/navigator'; const ROOT_CACHE_KEY = 'mmCourse:'; @@ -176,10 +177,14 @@ export class CoreCourseProvider { * @param courseId Course ID. * @return Whether the current view is a certain course. */ - // eslint-disable-next-line @typescript-eslint/no-unused-vars currentViewIsCourse(courseId: number): boolean { - // @todo implement - return false; + const route = CoreNavigator.getCurrentRoute({ routeData: { isCourseIndex: true } }); + + if (!route) { + return false; + } + + return Number(route.snapshot.params.courseId) == courseId; } /** diff --git a/src/core/features/grades/services/grades-helper.ts b/src/core/features/grades/services/grades-helper.ts index 5f7b9078d..baf9cfbe8 100644 --- a/src/core/features/grades/services/grades-helper.ts +++ b/src/core/features/grades/services/grades-helper.ts @@ -34,6 +34,7 @@ import { CoreDomUtils } from '@services/utils/dom'; import { CoreNavigator } from '@services/navigator'; import { makeSingleton, Translate } from '@singletons'; import { CoreError } from '@classes/errors/error'; +import { CoreCourseHelper } from '@features/course/services/course-helper'; /** * Service that provides some features regarding grades information. @@ -458,14 +459,13 @@ export class CoreGradesHelperProvider { siteId?: string, ): Promise { const modal = await CoreDomUtils.showModalLoading(); - let currentUserId: number; + + const site = await CoreSites.getSite(siteId); + + siteId = site.id; + const currentUserId = site.getUserId(); try { - const site = await CoreSites.getSite(siteId); - - siteId = site.id; - currentUserId = site.getUserId(); - if (!moduleId) { throw new CoreError('Invalid moduleId'); } @@ -477,27 +477,27 @@ export class CoreGradesHelperProvider { throw new CoreError('No grades found.'); } + // Can get grades. Do it. + const items = await CoreGrades.getGradeItems(courseId, userId, undefined, siteId); + + // Find the item of the module. + const item = Array.isArray(items) && items.find((item) => moduleId == item.cmid); + + if (!item) { + throw new CoreError('Grade item not found.'); + } + + // Open the item directly. + const gradeId = item.id; + + await CoreUtils.ignoreErrors( + CoreNavigator.navigateToSitePath(`/grades/${courseId}/${gradeId}`, { + siteId, + params: { userId }, + }), + ); + } catch (error) { try { - // Can get grades. Do it. - const items = await CoreGrades.getGradeItems(courseId, userId, undefined, siteId); - - // Find the item of the module. - const item = Array.isArray(items) && items.find((item) => moduleId == item.cmid); - - if (!item) { - throw new CoreError('Grade item not found.'); - } - - // Open the item directly. - const gradeId = item.id; - - await CoreUtils.ignoreErrors( - CoreNavigator.navigateToSitePath(`/grades/${courseId}/${gradeId}`, { - siteId, - params: { userId }, - }), - ); - } catch (error) { // Cannot get grade items or there's no need to. if (userId && userId != currentUserId) { // View another user grades. Open the grades page directly. @@ -517,24 +517,12 @@ export class CoreGradesHelperProvider { return; } - // @todo // Open the course with the grades tab selected. - // await CoreCourseHelper.getCourse(courseId, siteId).then(async (result) => { - // const pageParams = { - // course: result.course, - // selectedTab: 'CoreGrades', - // }; - - // // CoreContentLinksHelper.goInSite(navCtrl, 'CoreCourseSectionPage', pageParams, siteId) - // return await CoreUtils.ignoreErrors(CoreNavigator.navigateToSitePath('/course', { - // siteId, - // params: pageParams, - // })); - // }); + await CoreCourseHelper.getAndOpenCourse(courseId, { selectedTab: 'CoreGrades' }, siteId); + } catch (error) { + // Cannot get course for some reason, just open the grades page. + await CoreNavigator.navigateToSitePath(`/grades/${courseId}`, { siteId }); } - } catch (error) { - // Cannot get course for some reason, just open the grades page. - await CoreNavigator.navigateToSitePath(`/grades/${courseId}`, { siteId }); } finally { modal.dismiss(); } diff --git a/src/core/services/navigator.ts b/src/core/services/navigator.ts index dbd7cdee7..c5e380113 100644 --- a/src/core/services/navigator.ts +++ b/src/core/services/navigator.ts @@ -64,6 +64,7 @@ export type CoreNavigatorCurrentRouteOptions = Partial<{ params: Params; // Params to get the value from. route: ActivatedRoute; // Current Route. pageComponent: unknown; + routeData: Record; }>; /** @@ -401,18 +402,22 @@ export class CoreNavigatorService { */ getCurrentRoute(): ActivatedRoute; getCurrentRoute(options: CoreNavigatorCurrentRouteOptions): ActivatedRoute | null; - getCurrentRoute({ route, pageComponent }: CoreNavigatorCurrentRouteOptions = {}): ActivatedRoute | null { + getCurrentRoute({ route, pageComponent, routeData }: CoreNavigatorCurrentRouteOptions = {}): ActivatedRoute | null { route = route ?? Router.routerState.root; if (pageComponent && route.component === pageComponent) { return route; } - if (route.firstChild) { - return this.getCurrentRoute({ route: route.firstChild, pageComponent }); + if (routeData && CoreUtils.basicLeftCompare(routeData, route.snapshot.data, 3)) { + return route; } - return pageComponent ? null : route; + if (route.firstChild) { + return this.getCurrentRoute({ route: route.firstChild, pageComponent, routeData }); + } + + return pageComponent || routeData ? null : route; } /** diff --git a/src/core/services/utils/utils.ts b/src/core/services/utils/utils.ts index 13e1e9e47..1d267b7ce 100644 --- a/src/core/services/utils/utils.ts +++ b/src/core/services/utils/utils.ts @@ -176,7 +176,7 @@ export class CoreUtilsProvider { maxLevels: number = 0, level: number = 0, undefinedIsNull: boolean = true, - ): boolean | undefined { + ): boolean { if (typeof itemA == 'function' || typeof itemB == 'function') { return true; // Don't compare functions. } else if (typeof itemA == 'object' && typeof itemB == 'object') { @@ -189,7 +189,7 @@ export class CoreUtilsProvider { const value = itemA[name]; if (name == '$$hashKey') { // Ignore $$hashKey property since it's a "calculated" property. - return; + continue; } if (!this.basicLeftCompare(value, itemB[name], maxLevels, level + 1)) {