Merge pull request #4249 from crazyserver/MOBILE-4653

MOBILE-4653 chore: Improve formatPixelsSize to admit more units

src/addons/mod/data/fields/picture/component/picture.ts

@@ -16,7 +16,7 @@ import { AddonModDataEntryField } from '@addons/mod/data/services/data';
 import { Component } from '@angular/core';
 import { CoreFileEntry, CoreFileHelper } from '@services/file-helper';
 import { CoreFileSession } from '@services/file-session';
-import { CoreDomUtils } from '@services/utils/dom';
+import { CoreDom } from '@singletons/dom';
 import { AddonModDataFieldPluginBaseComponent } from '../../../classes/base-field-plugin-component';
 import { CoreFile } from '@services/file';
 import { ADDON_MOD_DATA_COMPONENT } from '@addons/mod/data/constants';
@@ -136,8 +136,8 @@ export class AddonModDataFieldPictureComponent extends AddonModDataFieldPluginBa
                 }
             }, 1);
 
-            this.width = CoreDomUtils.formatPixelsSize(this.field.param1);
+            this.width = CoreDom.formatSizeUnits(this.field.param1);
-            this.height = CoreDomUtils.formatPixelsSize(this.field.param2);
+            this.height = CoreDom.formatSizeUnits(this.field.param2);
         }
     }
 

src/addons/mod/lesson/pages/player/player.ts

@@ -57,6 +57,7 @@ import { ADDON_MOD_LESSON_COMPONENT, AddonModLessonJumpTo } from '../../constant
 import { CoreModals } from '@services/modals';
 import { CorePromiseUtils } from '@singletons/promise-utils';
 import { CoreWSError } from '@classes/errors/wserror';
+import { CoreDom } from '@singletons/dom';
 
 /**
  * Page that allows attempting and reviewing a lesson.
@@ -349,8 +350,8 @@ export class AddonModLessonPlayerPage implements OnInit, OnDestroy, CanLeave {
             await Promise.all(promises);
 
             this.mediaFile = this.lesson.mediafiles?.[0];
-            this.lessonWidth = this.lesson.slideshow ? CoreDomUtils.formatPixelsSize(this.lesson.mediawidth!) : '';
+            this.lessonWidth = this.lesson.slideshow ? CoreDom.formatSizeUnits(this.lesson.mediawidth!) : '';
-            this.lessonHeight = this.lesson.slideshow ? CoreDomUtils.formatPixelsSize(this.lesson.mediaheight!) : '';
+            this.lessonHeight = this.lesson.slideshow ? CoreDom.formatSizeUnits(this.lesson.mediaheight!) : '';
 
             await this.launchRetake(this.currentPage);
 

src/core/components/iframe/iframe.ts

@@ -29,6 +29,7 @@ import { filter } from 'rxjs/operators';
 import { NavigationStart } from '@angular/router';
 import { CoreSites } from '@services/sites';
 import { toBoolean } from '@/core/transforms/boolean';
+import { CoreDom } from '@singletons/dom';
 
 @Component({
     selector: 'core-iframe',
@@ -162,10 +163,10 @@ export class CoreIframeComponent implements OnChanges, OnDestroy {
      */
     async ngOnChanges(changes: {[name: string]: SimpleChange }): Promise<void> {
         if (changes.iframeWidth) {
-            this.iframeWidth = (this.iframeWidth && CoreDomUtils.formatPixelsSize(this.iframeWidth)) || '100%';
+            this.iframeWidth = (this.iframeWidth && CoreDom.formatSizeUnits(this.iframeWidth)) || '100%';
         }
         if (changes.iframeHeight) {
-            this.iframeHeight = (this.iframeHeight && CoreDomUtils.formatPixelsSize(this.iframeHeight)) || '100%';
+            this.iframeHeight = (this.iframeHeight && CoreDom.formatSizeUnits(this.iframeHeight)) || '100%';
         }
 
         if (!changes.src) {

src/core/services/utils/dom.ts

@@ -57,6 +57,7 @@ import { CoreLoadings } from '@services/loadings';
 import { CoreErrorHelper, CoreErrorObject } from '@services/error-helper';
 import { convertTextToHTMLElement } from '@/core/utils/create-html-element';
 import { CoreHTMLClasses } from '@singletons/html-classes';
+import { CoreDom } from '@singletons/dom';
 
 /*
  * "Utils" service with helper functions for UI, DOM elements and HTML code.
@@ -350,23 +351,10 @@ export class CoreDomUtilsProvider {
      *
      * @param size Size to format.
      * @returns Formatted size. If size is not valid, returns an empty string.
+     * @deprecated since 5.0. Use CoreDom.formatSizeUnits directly instead.
      */
     formatPixelsSize(size: string | number): string {
-        if (typeof size == 'string' && (size.indexOf('px') > -1 || size.indexOf('%') > -1 || size == 'auto' || size == 'initial')) {
+        return CoreDom.formatSizeUnits(size);
-            // It seems to be a valid size.
-            return size;
-        }
-
-        if (typeof size == 'string') {
-            // It's important to use parseInt instead of Number because Number('') is 0 instead of NaN.
-            size = parseInt(size, 10);
-        }
-
-        if (!isNaN(size)) {
-            return size + 'px';
-        }
-
-        return '';
     }
 
     /**

src/core/singletons/dom.ts

@@ -739,6 +739,48 @@ export class CoreDom {
         return css.replace(regExp, prefix + ' $1 $2');
     }
 
+    /**
+     * Formats a size to be used as width/height of an element.
+     * If the size is already valid (like '500px' or '50%') it won't be modified.
+     * Returned size will have a format like '500px'.
+     *
+     * @param size Size to format.
+     * @returns Formatted size. If size is not valid, returns an empty string.
+     */
+    static formatSizeUnits(size: string | number): string {
+        // Check for valid pixel units.
+        if (typeof size === 'string') {
+            size = size.replace(/ /g, ''); // Trim and remove all spaces.
+            if (CoreDom.hasValidSizeUnits(size) || size === 'auto' || size === 'initial' || size === 'inherit') {
+                // It seems to be a valid size.
+                return size;
+            }
+
+            // It's important to use parseInt instead of Number because Number('') is 0 instead of NaN.
+            size = parseInt(size, 10);
+        }
+
+        if (!isNaN(size)) {
+            return `${size}px`;
+        }
+
+        return '';
+    }
+
+    /**
+     * Check if a size has valid pixel units.
+     *
+     * @param size Size to check.
+     * @returns Whether the size has valid pixel units.
+     */
+    protected static hasValidSizeUnits(size: string): boolean {
+        const validUnits = ['px', '%', 'em', 'rem', 'cm', 'mm', 'in', 'pt', 'pc', 'ex', 'ch', 'vw', 'vh', 'vmin', 'vmax'];
+
+        const units = size.match('^[0-9]*\\.?[0-9]+(' + validUnits.join('|') + ')$');
+
+        return !!units && units.length > 1;
+    }
+
 }
 
 /**

src/core/singletons/tests/dom.test.ts

@@ -0,0 +1,43 @@
+// (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 { CoreDom } from '@singletons/dom';
+
+describe('CoreDom singleton', () => {
+
+    it('should return valid size with usual units', () => {
+        expect(CoreDom.formatSizeUnits(500)).toBe('500px');
+        expect(CoreDom.formatSizeUnits('500')).toBe('500px');
+        expect(CoreDom.formatSizeUnits('500px')).toBe('500px');
+        expect(CoreDom.formatSizeUnits('50%')).toBe('50%');
+    });
+
+    it('should return valid size with units', () => {
+        expect(CoreDom.formatSizeUnits('2 em')).toBe('2em');
+        expect(CoreDom.formatSizeUnits('1.5rem ')).toBe('1.5rem');
+    });
+
+    it('should return valid size with other values', () => {
+        expect(CoreDom.formatSizeUnits('auto')).toBe('auto');
+        expect(CoreDom.formatSizeUnits('initial')).toBe('initial');
+        expect(CoreDom.formatSizeUnits('inherit')).toBe('inherit');
+    });
+
+    it('should return empty string for invalid sizes', () => {
+        expect(CoreDom.formatSizeUnits('invalid')).toBe('');
+        expect(CoreDom.formatSizeUnits('em')).toBe('');
+        expect(CoreDom.formatSizeUnits(NaN)).toBe('');
+    });
+
+});