From d0e0ef86b6b807af95c2ca4ddc44e4e78ed75d25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pau=20Ferrer=20Oca=C3=B1a?= Date: Tue, 7 May 2024 14:25:43 +0200 Subject: [PATCH] MOBILE-4470 swiper: Improve how params are populated to fix Swiper bug --- .../mod/book/pages/contents/contents.ts | 2 - src/core/classes/tabs.ts | 54 +++++--------- .../components/swipe-slides/swipe-slides.ts | 56 +++++++-------- .../components/tabs-outlet/tabs-outlet.ts | 3 +- .../rich-text-editor/rich-text-editor.ts | 35 +++------- .../services/pushnotifications.ts | 2 +- .../settings/pages/general/general.ts | 3 + .../features/viewer/components/image/image.ts | 53 +++++--------- src/core/singletons/swiper.ts | 70 +++++++++++++++++++ 9 files changed, 146 insertions(+), 132 deletions(-) create mode 100644 src/core/singletons/swiper.ts diff --git a/src/addons/mod/book/pages/contents/contents.ts b/src/addons/mod/book/pages/contents/contents.ts index a1475bfaa..bff523109 100644 --- a/src/addons/mod/book/pages/contents/contents.ts +++ b/src/addons/mod/book/pages/contents/contents.ts @@ -41,7 +41,6 @@ import { } from '../../services/book'; import { CoreAnalytics, CoreAnalyticsEventType } from '@services/analytics'; import { CoreUrlUtils } from '@services/utils/url'; -import { IonicSlides } from '@ionic/angular'; /** * Page that displays a book contents. @@ -65,7 +64,6 @@ export class AddonModBookContentsPage implements OnInit, OnDestroy { displayNavBar = true; navigationItems: CoreNavigationBarItem[] = []; swiperOpts: CoreSwipeSlidesOptions = { - modules: [IonicSlides], autoHeight: true, observer: true, observeParents: true, diff --git a/src/core/classes/tabs.ts b/src/core/classes/tabs.ts index 2758cbaf6..aae11a6b0 100644 --- a/src/core/classes/tabs.ts +++ b/src/core/classes/tabs.ts @@ -17,7 +17,6 @@ import { Input, Output, EventEmitter, - OnInit, OnChanges, OnDestroy, AfterViewInit, @@ -28,7 +27,6 @@ import { import { BackButtonEvent } from '@ionic/core'; import { Subscription } from 'rxjs'; -import { Translate } from '@singletons'; import { CoreSettingsHelper } from '@features/settings/services/settings-helper'; import { CoreAriaRoleTab, CoreAriaRoleTabFindable } from './aria-role-tab'; import { CoreEventObserver } from '@singletons/events'; @@ -38,10 +36,9 @@ import { CoreError } from './errors/error'; import { CorePromisedValue } from './promised-value'; import { AsyncDirective } from './async-directive'; import { CoreDirectivesRegistry } from '@singletons/directives-registry'; -import { CorePlatform } from '@services/platform'; import { Swiper } from 'swiper'; import { SwiperOptions } from 'swiper/types'; -import { IonicSlides } from '@ionic/angular'; +import { CoreSwiper } from '@singletons/swiper'; /** * Class to abstract some common code for tabs. @@ -49,7 +46,7 @@ import { IonicSlides } from '@ionic/angular'; @Component({ template: '', }) -export class CoreTabsBaseComponent implements OnInit, AfterViewInit, OnChanges, OnDestroy, AsyncDirective { +export class CoreTabsBaseComponent implements AfterViewInit, OnChanges, OnDestroy, AsyncDirective { // Minimum tab's width. protected static readonly MIN_TAB_WIDTH = 107; @@ -59,32 +56,26 @@ export class CoreTabsBaseComponent implements OnInit, Aft @Output() protected ionChange = new EventEmitter(); // Emitted when the tab changes. protected swiper?: Swiper; - @ViewChild('swiperRef') - set swiperRef(swiperRef: ElementRef) { + @ViewChild('swiperRef') set swiperRef(swiperRef: ElementRef) { /** * This setTimeout waits for Ionic's async initialization to complete. * Otherwise, an outdated swiper reference will be used. */ setTimeout(() => { - if (swiperRef?.nativeElement?.swiper && !this.swiper) { - this.swiper = swiperRef.nativeElement.swiper as Swiper; - - this.swiper.changeLanguageDirection(CorePlatform.isRTL ? 'rtl' : 'ltr'); - - Object.keys(this.swiperOpts).forEach((key) => { - if (this.swiper) { - this.swiper.params[key] = this.swiperOpts[key]; - } - }); - - // Subscribe to changes. - this.swiper.on('slideChangeTransitionEnd', () => { - this.slideChanged(); - }); - - this.init(); + const swiper = CoreSwiper.initSwiperIfAvailable(this.swiper, swiperRef, this.swiperOpts); + if (!swiper) { + return; } - }, 0); + + this.swiper = swiper; + + // Subscribe to changes. + this.swiper.on('slideChangeTransitionEnd', () => { + this.slideChanged(); + }); + + this.init(); + }); } tabs: T[] = []; // List of tabs. @@ -97,7 +88,6 @@ export class CoreTabsBaseComponent implements OnInit, Aft numTabsShown = 0; description = ''; swiperOpts: SwiperOptions = { - modules: [IonicSlides], slidesPerView: 3, centerInsufficientSlides: true, threshold: 10, @@ -127,18 +117,6 @@ export class CoreTabsBaseComponent implements OnInit, Aft CoreDirectivesRegistry.register(element.nativeElement, this); } - /** - * @inheritdoc - */ - async ngOnInit(): Promise { - // Change the side when the language changes. - this.subscriptions.push(Translate.onLangChange.subscribe(() => { - setTimeout(() => { - this.swiper?.changeLanguageDirection(CorePlatform.isRTL ? 'rtl' : 'ltr'); - }); - })); - } - /** * @inheritdoc */ diff --git a/src/core/components/swipe-slides/swipe-slides.ts b/src/core/components/swipe-slides/swipe-slides.ts index 49690e299..a5f60f904 100644 --- a/src/core/components/swipe-slides/swipe-slides.ts +++ b/src/core/components/swipe-slides/swipe-slides.ts @@ -24,6 +24,7 @@ import { CoreUtils } from '@services/utils/utils'; import { CoreDom } from '@singletons/dom'; import { CoreEventObserver } from '@singletons/events'; import { CoreMath } from '@singletons/math'; +import { CoreSwiper } from '@singletons/swiper'; import { Swiper } from 'swiper'; import { SwiperOptions } from 'swiper/types'; /** @@ -42,29 +43,28 @@ export class CoreSwipeSlidesComponent implements OnChanges, OnDe @Output() onDidChange = new EventEmitter>(); protected swiper?: Swiper; - @ViewChild('swiperRef') - set swiperRef(swiperRef: ElementRef) { + @ViewChild('swiperRef') set swiperRef(swiperRef: ElementRef) { /** * This setTimeout waits for Ionic's async initialization to complete. * Otherwise, an outdated swiper reference will be used. */ setTimeout(async () => { - if (swiperRef?.nativeElement?.swiper) { - this.swiper = swiperRef.nativeElement.swiper as Swiper; - - await this.initialize(); - - if (this.options.initialSlide) { - this.swiper.slideTo(this.options.initialSlide, 0, this.options.runCallbacksOnInit); - } - - this.updateOptions(); - - this.swiper.on('slideChangeTransitionStart', () => this.slideWillChange()); - this.swiper.on('slideChangeTransitionEnd', () => this.slideDidChange()); - + const swiper = CoreSwiper.initSwiperIfAvailable(this.swiper, swiperRef, this.options); + if (!swiper) { + return; } - }, 0); + + this.swiper = swiper; + + await this.initialize(); + + if (this.options.initialSlide) { + this.swiper.slideTo(this.options.initialSlide, 0, this.options.runCallbacksOnInit); + } + + this.swiper.on('slideChangeTransitionStart', () => this.slideWillChange()); + this.swiper.on('slideChangeTransitionEnd', () => this.slideDidChange()); + }); } @ContentChild(TemplateRef) template?: TemplateRef<{item: Item; active: boolean}>; // Template defined by the content. @@ -173,17 +173,21 @@ export class CoreSwipeSlidesComponent implements OnChanges, OnDe // If slides are being updated, wait for the update to finish. await this.ready(); + if (!this.swiper) { + return; + } + // Verify that the number of slides matches the number of items. - const slidesLength = this.swiper?.slides?.length || 0; + const slidesLength = this.swiper.slides?.length || 0; if (slidesLength !== this.items.length) { // Number doesn't match, do a new update to try to match them. await this.updateSlidesComponent(); } - if (!this.swiper?.slides) { + if (!this.swiper.slides) { return; } - this.swiper?.slideTo(index, speed, runCallbacks); + this.swiper.slideTo(index, speed, runCallbacks); } /** @@ -195,7 +199,7 @@ export class CoreSwipeSlidesComponent implements OnChanges, OnDe */ async slideToItem(item: Item, speed?: number, runCallbacks?: boolean): Promise { const index = this.manager?.getSource().getItemIndex(item) ?? -1; - if (index != -1) { + if (index !== -1) { await this.slideToIndex(index, speed, runCallbacks); } } @@ -248,15 +252,7 @@ export class CoreSwipeSlidesComponent implements OnChanges, OnDe return; } - if (this.swiper.params === undefined) { - this.swiper.params = {}; - } - - Object.keys(this.options).forEach((key) => { - if (this.swiper) { - this.swiper.params[key] = this.options[key]; - } - }); + CoreSwiper.updateOptions(this.swiper, this.options); } /** diff --git a/src/core/components/tabs-outlet/tabs-outlet.ts b/src/core/components/tabs-outlet/tabs-outlet.ts index 34b6a2724..0deddc47e 100644 --- a/src/core/components/tabs-outlet/tabs-outlet.ts +++ b/src/core/components/tabs-outlet/tabs-outlet.ts @@ -15,7 +15,6 @@ import { Component, Input, - OnInit, OnChanges, OnDestroy, AfterViewInit, @@ -52,7 +51,7 @@ import { CoreDirectivesRegistry } from '@singletons/directives-registry'; styleUrls: ['../tabs/tabs.scss'], }) export class CoreTabsOutletComponent extends CoreTabsBaseComponent - implements OnInit, AfterViewInit, OnChanges, OnDestroy { + implements AfterViewInit, OnChanges, OnDestroy { /** * Determine tabs layout. diff --git a/src/core/features/editor/components/rich-text-editor/rich-text-editor.ts b/src/core/features/editor/components/rich-text-editor/rich-text-editor.ts index d35076d14..ee8486927 100644 --- a/src/core/features/editor/components/rich-text-editor/rich-text-editor.ts +++ b/src/core/features/editor/components/rich-text-editor/rich-text-editor.ts @@ -25,7 +25,7 @@ import { AfterViewInit, } from '@angular/core'; import { FormControl } from '@angular/forms'; -import { IonTextarea, IonContent, IonicSlides } from '@ionic/angular'; +import { IonTextarea, IonContent } from '@ionic/angular'; import { Subscription } from 'rxjs'; import { CoreSites } from '@services/sites'; @@ -33,7 +33,6 @@ import { CoreFilepool } from '@services/filepool'; import { CoreDomUtils } from '@services/utils/dom'; import { CoreUrlUtils } from '@services/utils/url'; import { CoreUtils } from '@services/utils/utils'; -import { Translate } from '@singletons'; import { CoreEventFormActionData, CoreEventObserver, CoreEvents } from '@singletons/events'; import { CoreEditorOffline } from '../../services/editor-offline'; import { CoreDirectivesRegistry } from '@singletons/directives-registry'; @@ -45,6 +44,7 @@ import { CorePlatform } from '@services/platform'; import { Swiper } from 'swiper'; import { SwiperOptions } from 'swiper/types'; import { ContextLevel } from '@/core/constants'; +import { CoreSwiper } from '@singletons/swiper'; /** * Component to display a rich text editor if enabled. @@ -81,26 +81,21 @@ export class CoreEditorRichTextEditorComponent implements OnInit, AfterViewInit, @ViewChild('editor') editor?: ElementRef; // WYSIWYG editor. @ViewChild('textarea') textarea?: IonTextarea; // Textarea editor. @ViewChild('toolbar') toolbar?: ElementRef; + protected toolbarSlides?: Swiper; - @ViewChild('swiperRef') - set swiperRef(swiperRef: ElementRef) { + @ViewChild('swiperRef') set swiperRef(swiperRef: ElementRef) { /** * This setTimeout waits for Ionic's async initialization to complete. * Otherwise, an outdated swiper reference will be used. */ setTimeout(() => { - if (swiperRef.nativeElement?.swiper) { - this.toolbarSlides = swiperRef.nativeElement.swiper as Swiper; - - this.toolbarSlides.changeLanguageDirection(CorePlatform.isRTL ? 'rtl' : 'ltr'); - - Object.keys(this.swiperOpts).forEach((key) => { - if (this.toolbarSlides) { - this.toolbarSlides.params[key] = this.swiperOpts[key]; - } - }); + const swiper = CoreSwiper.initSwiperIfAvailable(this.toolbarSlides, swiperRef, this.swiperOpts); + if (!swiper) { + return; } - }, 0); + + this.toolbarSlides = swiper; + }); } protected readonly DRAFT_AUTOSAVE_FREQUENCY = 30000; @@ -128,7 +123,6 @@ export class CoreEditorRichTextEditorComponent implements OnInit, AfterViewInit, protected originalContent?: string; protected resizeFunction?: () => Promise; protected selectionChangeFunction = (): void => this.updateToolbarStyles(); - protected languageChangedSubscription?: Subscription; protected resizeListener?: CoreEventObserver; protected domPromise?: CoreCancellablePromise; protected buttonsDomPromise?: CoreCancellablePromise; @@ -159,7 +153,6 @@ export class CoreEditorRichTextEditorComponent implements OnInit, AfterViewInit, isEmpty = true; swiperOpts: SwiperOptions = { - modules: [IonicSlides], slidesPerView: 6, centerInsufficientSlides: true, watchSlidesProgress: true, @@ -301,13 +294,6 @@ export class CoreEditorRichTextEditorComponent implements OnInit, AfterViewInit, // Check the height again, now the window height should have been updated. this.maximizeEditorSize(); }); - - // Change the side when the language changes. - this.languageChangedSubscription = Translate.onLangChange.subscribe(() => { - setTimeout(() => { - this.toolbarSlides?.changeLanguageDirection(CorePlatform.isRTL ? 'rtl' : 'ltr'); - }); - }); } /** @@ -1120,7 +1106,6 @@ export class CoreEditorRichTextEditorComponent implements OnInit, AfterViewInit, */ ngOnDestroy(): void { this.valueChangeSubscription?.unsubscribe(); - this.languageChangedSubscription?.unsubscribe(); document.removeEventListener('selectionchange', this.selectionChangeFunction); diff --git a/src/core/features/pushnotifications/services/pushnotifications.ts b/src/core/features/pushnotifications/services/pushnotifications.ts index 8a5a9d66e..e27c7746a 100644 --- a/src/core/features/pushnotifications/services/pushnotifications.ts +++ b/src/core/features/pushnotifications/services/pushnotifications.ts @@ -204,7 +204,7 @@ export class CorePushNotificationsProvider { protected async initializeDatabase(): Promise { try { await CoreApp.createTablesFromSchema(APP_SCHEMA); - } catch (e) { + } catch { // Ignore errors. } diff --git a/src/core/features/settings/pages/general/general.ts b/src/core/features/settings/pages/general/general.ts index 176a04772..c19b483a3 100644 --- a/src/core/features/settings/pages/general/general.ts +++ b/src/core/features/settings/pages/general/general.ts @@ -168,6 +168,9 @@ export class CoreSettingsGeneralPage { /** * Apply language changes and restart the app. + * + * IMPORTANT NOTE: If for any reason we decide to remove this method, + * we'll need to listen to lang change on Slides to change direction. */ protected async applyLanguageAndRestart(): Promise { // Invalidate cache for all sites to get the content in the right language. diff --git a/src/core/features/viewer/components/image/image.ts b/src/core/features/viewer/components/image/image.ts index cf1f236d5..f52e9ab52 100644 --- a/src/core/features/viewer/components/image/image.ts +++ b/src/core/features/viewer/components/image/image.ts @@ -17,7 +17,7 @@ import { ModalController, Translate } from '@singletons'; import { CoreMath } from '@singletons/math'; import { Swiper } from 'swiper'; import { SwiperOptions } from 'swiper/types'; -import { IonicSlides } from '@ionic/angular'; +import { CoreSwiper } from '@singletons/swiper'; /** * Modal component to view an image. @@ -30,23 +30,21 @@ import { IonicSlides } from '@ionic/angular'; export class CoreViewerImageComponent implements OnInit { protected swiper?: Swiper; - @ViewChild('swiperRef') - set swiperRef(swiperRef: ElementRef) { + @ViewChild('swiperRef') set swiperRef(swiperRef: ElementRef) { /** * This setTimeout waits for Ionic's async initialization to complete. * Otherwise, an outdated swiper reference will be used. */ setTimeout(() => { - if (swiperRef.nativeElement?.swiper) { - this.swiper = swiperRef.nativeElement.swiper as Swiper; - - Object.keys(this.swiperOpts).forEach((key) => { - if (this.swiper) { - this.swiper.params[key] = this.swiperOpts[key]; - } - }); + const swiper = CoreSwiper.initSwiperIfAvailable(this.swiper, swiperRef, this.swiperOpts); + if (!swiper) { + return; } - }, 0); + + this.swiper = swiper; + + this.swiper.zoom.enable(); + }); } @Input() title = ''; // Modal title. @@ -55,24 +53,20 @@ export class CoreViewerImageComponent implements OnInit { @Input() componentId?: string | number; // Component ID to use in external-content. private static readonly MAX_RATIO = 8; + private static readonly MIN_RATIO = 0.5; protected swiperOpts: SwiperOptions = { - modules: [IonicSlides], freeMode: true, slidesPerView: 1, centerInsufficientSlides: true, centeredSlides: true, zoom: { maxRatio: CoreViewerImageComponent.MAX_RATIO, - minRatio: 0.5, // User can zoom out to 0.5 only using pinch gesture. + minRatio: CoreViewerImageComponent.MIN_RATIO, + toggle: true, }, }; - protected zoomRatio = 1; - - constructor(protected element: ElementRef) { - } - /** * @inheritdoc */ @@ -93,27 +87,18 @@ export class CoreViewerImageComponent implements OnInit { * @param zoomIn True to zoom in, false to zoom out. */ zoom(zoomIn = true): void { - const imageElement = this.element.nativeElement.querySelector('img'); - - if (!this.swiper || !imageElement) { + if (!this.swiper) { return; } + let zoomRatio = this.swiper.zoom.scale; zoomIn - ? this.zoomRatio *= 2 - : this.zoomRatio /= 2; + ? zoomRatio *= 2 + : zoomRatio /= 2; - // Using 1 as minimum for manual zoom. - this.zoomRatio = CoreMath.clamp(this.zoomRatio, 1, CoreViewerImageComponent.MAX_RATIO); + zoomRatio = CoreMath.clamp(zoomRatio, CoreViewerImageComponent.MIN_RATIO, CoreViewerImageComponent.MAX_RATIO); - if (this.zoomRatio > 1) { - this.swiper.zoom.in(); - - imageElement.style.transform = - 'translate3d(0px, 0px, 0px) scale(' + this.zoomRatio + ')'; - } else { - this.swiper.zoom.out(); - } + this.swiper.zoom.in(zoomRatio); } } diff --git a/src/core/singletons/swiper.ts b/src/core/singletons/swiper.ts new file mode 100644 index 000000000..46cb7bb3d --- /dev/null +++ b/src/core/singletons/swiper.ts @@ -0,0 +1,70 @@ +// (C) Copyright 2015 Moodle Pty Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { ElementRef } from '@angular/core'; +import { IonicSlides } from '@ionic/angular'; +import { CorePlatform } from '@services/platform'; +import Swiper from 'swiper'; +import { SwiperOptions } from 'swiper/types'; + +/** + * Singleton with helper functions for SwiperJS. + */ +export class CoreSwiper { + + /** + * Initialize a Swiper instance. + * It will return swiper instance if current is not set or destroyed and new is set and not destroyed. + * + * @param currentSwiper Current Swiper instance. + * @param newSwiperRef New Swiper Element Ref. + * @param swiperOpts Swiper options. + * @returns Initialized Swiper instance. + */ + static initSwiperIfAvailable( + currentSwiper?: Swiper, + newSwiperRef?: ElementRef, + swiperOpts?: SwiperOptions, + ): Swiper | undefined { + const swiper = newSwiperRef?.nativeElement?.swiper as Swiper | undefined; + if (!swiper || swiper.destroyed || (currentSwiper && !currentSwiper.destroyed)) { + return; + } + + Swiper.use([IonicSlides]); + + CoreSwiper.updateOptions(swiper, swiperOpts); + + swiper.changeLanguageDirection(CorePlatform.isRTL ? 'rtl' : 'ltr'); + + return swiper; + } + + /** + * Update Swiper options. + * + * @param swiper Swiper instance. + * @param swiperOpts Swiper options. + */ + static updateOptions(swiper: Swiper, swiperOpts?: SwiperOptions): void { + if (!swiperOpts) { + return; + } + + Object.assign(swiper.el, swiperOpts); + + swiper.update(); + } + +}