fix(module: modal): repeated cleaning dom (#3665) (#3673)

* fix: repeated cleaning dom * fix: OOM * fix build * fix lint * remove the event handler * remove handler after element was deleted --------- Co-authored-by: James Yeung <shunjiey@hotmail.com>
2024-11-29 10:38:18 +08:00 · 2024-02-28 23:29:48 +08:00 · 2024-02-28 23:29:48 +08:00 · bfb9e3a56f
commit bfb9e3a56f
parent 28e195806d
9 changed files with 227 additions and 148 deletions
--- a/components/.eslintrc
+++ b/components/.eslintrc
@ -14,7 +14,12 @@
    "indent": [
      "error",
      2,
-      { "ignoreComments": true }
+      {
+        "ignoreComments": true,
+        "SwitchCase": 1,
+        "flatTernaryExpressions": false,
+        "offsetTernaryExpressions": false
+      }
    ]
  }
 }
--- a/components/core/JsInterop/modules/components/overlay.ts
+++ b/components/core/JsInterop/modules/components/overlay.ts
@ -238,56 +238,56 @@ export class Overlay {
  static setVerticalCalculation(placement: Placement, position: "top" | "bottom") {
    if (position === "top") {
      switch (placement) {
-      case Placement.LeftTop:
-      case Placement.RightTop:          
-        return function(triggerTop: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) {               
-          return { 
-            top: triggerTop,
-            bottom: Overlay.reversePositionValue(triggerTop, container.scrollHeight, overlayHeight) 
+        case Placement.LeftTop:
+        case Placement.RightTop:          
+          return function(triggerTop: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) {               
+            return { 
+              top: triggerTop,
+              bottom: Overlay.reversePositionValue(triggerTop, container.scrollHeight, overlayHeight) 
+            };              
          }; 
-        }; 
-      case Placement.BottomLeft:
-      case Placement.Bottom:
-      case Placement.BottomRight:
-        return function(triggerTop: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) { 
-          const position: verticalPosition = {
-            top: triggerTop + triggerHeight + constraints.verticalOffset,
+        case Placement.BottomLeft:
+        case Placement.Bottom:
+        case Placement.BottomRight:
+          return function(triggerTop: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) { 
+            const position: verticalPosition = {
+              top: triggerTop + triggerHeight + constraints.verticalOffset,
+            };
+            position.bottom = Overlay.reversePositionValue(position.top, container.scrollHeight, overlayHeight)
+            return position;
          }; 
-          position.bottom = Overlay.reversePositionValue(position.top, container.scrollHeight, overlayHeight)
-          return position;
-        }; 
-      case Placement.Left:
-      case Placement.Right:
-        return function(triggerTop: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) {     
-          const position: verticalPosition = { 
-            top: triggerTop + (triggerHeight / 2) - (overlayHeight / 2)
+        case Placement.Left:
+        case Placement.Right:
+          return function(triggerTop: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) {     
+            const position: verticalPosition = { 
+              top: triggerTop + (triggerHeight / 2) - (overlayHeight / 2)
+            };
+            position.bottom = Overlay.reversePositionValue(position.top, container.scrollHeight, overlayHeight)
+            return position;
          }; 
-          position.bottom = Overlay.reversePositionValue(position.top, container.scrollHeight, overlayHeight)
-          return position;
-        }; 
      }
    }    
    if (position === "bottom") {
      switch (placement) {
-      case Placement.TopLeft:
-      case Placement.Top:
-      case Placement.TopRight:
-        return function(triggerBottom: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) { 
-          const position: verticalPosition = { 
-            bottom: triggerBottom + triggerHeight + constraints.verticalOffset,
+        case Placement.TopLeft:
+        case Placement.Top:
+        case Placement.TopRight:
+          return function(triggerBottom: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) { 
+            const position: verticalPosition = { 
+              bottom: triggerBottom + triggerHeight + constraints.verticalOffset,
+            };
+            position.top = Overlay.reversePositionValue(position.bottom, container.scrollHeight, overlayHeight);
+            return position;
          };           
-          position.top = Overlay.reversePositionValue(position.bottom, container.scrollHeight, overlayHeight);
-          return position;
-        };           
-      case Placement.LeftBottom:
-      case Placement.RightBottom:
-        return function(triggerBottom: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) {  
-          const position: verticalPosition = { 
-            bottom: triggerBottom,
-            top: Overlay.reversePositionValue(triggerBottom, container.scrollHeight, overlayHeight)
+        case Placement.LeftBottom:
+        case Placement.RightBottom:
+          return function(triggerBottom: number, triggerHeight: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayHeight: number, constraints: overlayConstraints) {  
+            const position: verticalPosition = { 
+              bottom: triggerBottom,
+              top: Overlay.reversePositionValue(triggerBottom, container.scrollHeight, overlayHeight)
+            };            
+            return position;                               
          };   
-          return position;                               
-        };   
      }
    }
    //fallback - should not happen, but to avoid crashing scenarios, revert to BottomLeft
@ -298,57 +298,57 @@ export class Overlay {
  static setHorizontalCalculation(placement: Placement, position: "left" | "right") {
    if (position === "left") {
      switch (placement) {
-      case Placement.TopLeft:
-      case Placement.BottomLeft:
-        return function(triggerLeft: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {        
-          return { 
-            left: triggerLeft,
-            right: Overlay.reversePositionValue(triggerLeft, container.scrollWidth, overlayWidth)
+        case Placement.TopLeft:
+        case Placement.BottomLeft:
+          return function(triggerLeft: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {        
+            return { 
+              left: triggerLeft,
+              right: Overlay.reversePositionValue(triggerLeft, container.scrollWidth, overlayWidth)
+            };              
          }; 
-        }; 
-      case Placement.Right:
-      case Placement.RightTop:
-      case Placement.RightBottom:
-        return function(triggerLeft: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {        
-          const position: horizontalPosition = {
-            left: triggerLeft + triggerWidth + constraints.horizontalOffset
+        case Placement.Right:
+        case Placement.RightTop:
+        case Placement.RightBottom:
+          return function(triggerLeft: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {        
+            const position: horizontalPosition = {
+              left: triggerLeft + triggerWidth + constraints.horizontalOffset
+            };
+            position.right = Overlay.reversePositionValue(position.left, container.scrollWidth, overlayWidth)
+            return position;
          };
-          position.right = Overlay.reversePositionValue(position.left, container.scrollWidth, overlayWidth)
-          return position;
-        };

-      case Placement.Top:
-      case Placement.Bottom:
-        return function(triggerLeft: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {        
-          const position: horizontalPosition = {
-            left: triggerLeft + (triggerWidth / 2) - (overlayWidth / 2)
+        case Placement.Top:
+        case Placement.Bottom:
+          return function(triggerLeft: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {        
+            const position: horizontalPosition = {
+              left: triggerLeft + (triggerWidth / 2) - (overlayWidth / 2)
+            };
+            position.right = Overlay.reversePositionValue(position.left, container.scrollWidth, overlayWidth)
+            return position;
          };
-          position.right = Overlay.reversePositionValue(position.left, container.scrollWidth, overlayWidth)
-          return position;
-        };
      }
    }
    if (position === "right") {
      switch (placement) {
-      case Placement.TopRight:
-      case Placement.BottomRight:
-        return function(triggerRight: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {        
-          const position: horizontalPosition = {
-            right: triggerRight,
-            left: Overlay.reversePositionValue(triggerRight, container.scrollWidth, overlayWidth)
+        case Placement.TopRight:
+        case Placement.BottomRight:
+          return function(triggerRight: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {        
+            const position: horizontalPosition = {
+              right: triggerRight,
+              left: Overlay.reversePositionValue(triggerRight, container.scrollWidth, overlayWidth)
+            };            
+            return position;
          };
-          return position;
-        };
-      case Placement.Left:
-      case Placement.LeftTop:
-      case Placement.LeftBottom:
-        return function(triggerRight: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {
-          const position: horizontalPosition = {
-            right: triggerRight + triggerWidth + constraints.horizontalOffset
+        case Placement.Left:
+        case Placement.LeftTop:
+        case Placement.LeftBottom:
+          return function(triggerRight: number, triggerWidth: number, container: domTypes.domInfo, trigger: domTypes.domInfo, overlayWidth: number, constraints: overlayConstraints) {
+            const position: horizontalPosition = {
+              right: triggerRight + triggerWidth + constraints.horizontalOffset
+            };
+            position.left = Overlay.reversePositionValue(position.right, container.scrollWidth, overlayWidth)
+            return position;
          };          
-          position.left = Overlay.reversePositionValue(position.right, container.scrollWidth, overlayWidth)
-          return position;
-        };          
      }      
    }
    //fallback - should not happen, but to avoid crashing scenarios, revert to BottomLeft
--- a/components/core/JsInterop/modules/dom/manipulationHelper.ts
+++ b/components/core/JsInterop/modules/dom/manipulationHelper.ts
@ -1,8 +1,7 @@
-import { domInfoHelper } from './exports'
-import { styleHelper } from '../styleHelper'
-import { state } from '../stateProvider'
-import * as enums from '../enums';
-
+import { domInfoHelper } from "./exports";
+import { styleHelper } from "../styleHelper";
+import { state } from "../stateProvider";
+import * as enums from "../enums";

 let cachedScrollBarSize: number | undefined = undefined;
 const scrollIds = new Map<HTMLElement, number>();
@ -75,11 +74,14 @@ export class manipulationHelper {
      this.fallbackCopyTextToClipboard(text);
      return;
    }
-    navigator.clipboard.writeText(text).then(function () {
-      console.log('Async: Copying to clipboard was successful!');
-    }, function (err) {
-      console.error('Async: Could not copy text: ', err);
-    });
+    navigator.clipboard.writeText(text).then(
+      function () {
+        console.log("Async: Copying to clipboard was successful!");
+      },
+      function (err) {
+        console.error("Async: Could not copy text: ", err);
+      }
+    );
  }

  private static fallbackCopyTextToClipboard(text) {
@ -100,7 +102,7 @@ export class manipulationHelper {
      const msg = successful ? 'successful' : 'unsuccessful';
      console.log('Fallback: Copying text command was ' + msg);
    } catch (err) {
-      console.error('Fallback: Oops, unable to copy', err);
+      console.error("Fallback: Oops, unable to copy", err);
    }

    document.body.removeChild(textArea);
@ -108,29 +110,29 @@ export class manipulationHelper {

  static focus(selector, noScroll: boolean = false, option: enums.FocusBehavior = enums.FocusBehavior.FocusAtLast) {
    const dom = domInfoHelper.get(selector);
+
    if (!(dom instanceof HTMLElement))
      throw new Error("Unable to focus on invalid element.");

    dom.focus({
-      preventScroll: noScroll
+      preventScroll: noScroll,
    });

    if (dom instanceof HTMLInputElement || dom instanceof HTMLTextAreaElement) {
      switch (option) {
-      case enums.FocusBehavior.FocusAndSelectAll:
-        dom.select();
-        break;
-      case enums.FocusBehavior.FocusAtFirst:
-        dom.setSelectionRange(0, 0);
-        break;
-      case enums.FocusBehavior.FocusAtLast:
-        dom.setSelectionRange(-1, -1);
-        break;
+        case enums.FocusBehavior.FocusAndSelectAll:
+          dom.select();
+          break;
+        case enums.FocusBehavior.FocusAtFirst:
+          dom.setSelectionRange(0, 0);
+          break;
+        case enums.FocusBehavior.FocusAtLast:
+          dom.setSelectionRange(-1, -1);
+          break;
      }
    }
  }

-
  static blur(selector) {
    const dom = domInfoHelper.get(selector);
    if (dom) {
@ -147,7 +149,11 @@ export class manipulationHelper {
    }
  }

-  static smoothScrollTo(selector: Element | string, parentElement: HTMLElement, duration: number) {
+  static smoothScrollTo(
+    selector: Element | string,
+    parentElement: HTMLElement,
+    duration: number
+  ) {
    const element = domInfoHelper.get(selector);
    const to = element.offsetTop;
    if (scrollIds.get(parentElement)) {
@ -180,13 +186,20 @@ export class manipulationHelper {

  static slideTo(targetPageY) {
    const timer = setInterval(function () {
-      const currentY = document.documentElement.scrollTop || document.body.scrollTop;
-      const distance = targetPageY > currentY ? targetPageY - currentY : currentY - targetPageY;
+      const currentY =
+        document.documentElement.scrollTop || document.body.scrollTop;
+      const distance =
+        targetPageY > currentY
+          ? targetPageY - currentY
+          : currentY - targetPageY;
      const speed = Math.ceil(distance / 10);
      if (currentY === targetPageY) {
        clearInterval(timer);
      } else {
-        window.scrollTo(0, targetPageY > currentY ? currentY + speed : currentY - speed);
+        window.scrollTo(
+          0,
+          targetPageY > currentY ? currentY + speed : currentY - speed
+        );
      }
    }, 10);
  }
@ -216,12 +229,14 @@ export class manipulationHelper {
    });
    state.oldBodyCacheStack.push(oldBodyCache);
    const scrollBarSize = this.getScrollBarSize();
-    styleHelper.css(body,
-      {
-        "position": "relative",
-        "width": this.hasScrollbar() && scrollBarSize > 0 ? `calc(100% - ${scrollBarSize}px)` : null,
-        "overflow": "hidden"
-      });
+    styleHelper.css(body, {
+      position: "relative",
+      width:
+        this.hasScrollbar() && scrollBarSize > 0
+          ? `calc(100% - ${scrollBarSize}px)`
+          : null,
+      overflow: "hidden",
+    });
    styleHelper.addCls(document.body, "ant-scrolling-effect");
  }

@ -229,9 +244,8 @@ export class manipulationHelper {
    if (force) {
      state.oldBodyCacheStack = [];
    }
+
    const oldBodyCache = state.oldBodyCacheStack.length > 0 ? state.oldBodyCacheStack.pop() : {};
-    
-
    styleHelper.css(document.body,
      {
        "position": oldBodyCache["position"] ?? null,
@ -244,9 +258,11 @@ export class manipulationHelper {
  static hasScrollbar = () => {
    const overflow = document.body.style.overflow;
    if (overflow && overflow === "hidden") return false;
-    return document.body.scrollHeight > (window.innerHeight || document.documentElement.clientHeight);
-  }
-
+    return (
+      document.body.scrollHeight >
+      (window.innerHeight || document.documentElement.clientHeight)
+    );
+  };

  /**
   * getScrollBarSize
--- a/components/drawer/DrawerContainer.razor.cs
+++ b/components/drawer/DrawerContainer.razor.cs
@ -11,11 +11,22 @@ namespace AntDesign
        [Inject]
        private DrawerService DrawerService { get; set; }

+        [Inject]
+        private NavigationManager NavigationManager { get; set; }
+
        protected override void OnInitialized()
        {
            DrawerService.OnCloseEvent += DrawerService_OnClose;
            DrawerService.OnOpenEvent += DrawerService_OnCreate;
            DrawerService.OnUpdateEvent += DrawerService_OnUpdateEvent;
+
+            NavigationManager.LocationChanged += OnLocationChanged;
+        }
+
+        private void OnLocationChanged(object sender, EventArgs e)
+        {
+            _drawerRefs.Clear();
+            InvokeStateHasChanged();
        }

        protected override void Dispose(bool disposing)
@ -23,6 +34,7 @@ namespace AntDesign
            DrawerService.OnCloseEvent -= DrawerService_OnClose;
            DrawerService.OnOpenEvent -= DrawerService_OnCreate;
            DrawerService.OnUpdateEvent -= DrawerService_OnUpdateEvent;
+            NavigationManager.LocationChanged -= OnLocationChanged;

            base.Dispose(disposing);
        }
--- a/components/modal/confirmDialog/ComfirmContainer.razor.cs
+++ b/components/modal/confirmDialog/ComfirmContainer.razor.cs
@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Components;

@ -12,6 +13,9 @@ namespace AntDesign
        [Inject]
        private ConfirmService ConfirmService { get; set; }

+        [Inject]
+        private NavigationManager NavigationManager { get; set; }
+
        private readonly List<ConfirmRef> _confirmRefs = new List<ConfirmRef>();

        #region override
@ -27,10 +31,18 @@ namespace AntDesign
            ModalService.OnConfirmUpdateEvent += OnConfirmUpdate;

            ConfirmService.OnOpenEvent += OnConfirmOpen;
+
+            NavigationManager.LocationChanged += OnLocationChanged;
        }

        #endregion

+        private void OnLocationChanged(object sender, EventArgs e)
+        {
+            _confirmRefs.Clear();
+            InvokeStateHasChanged();
+        }
+
        /// <summary>
        /// create and open a Confirm dialog
        /// </summary>
@ -116,6 +128,7 @@ namespace AntDesign
            ModalService.OnConfirmUpdateEvent -= OnConfirmUpdate;

            ConfirmService.OnOpenEvent -= OnConfirmOpen;
+            NavigationManager.LocationChanged -= OnLocationChanged;

            base.Dispose(disposing);
        }
--- a/components/modal/core/Dialog.razor.cs
+++ b/components/modal/core/Dialog.razor.cs
@ -16,9 +16,6 @@ namespace AntDesign
    {
        private const string IdPrefix = "Ant-Design-";

-        [Inject]
-        private NavigationManager NavigationManager { get; set; }
-
        #region Parameters

 #pragma warning disable 1591
@ -397,15 +394,6 @@ namespace AntDesign

        #region override

-        protected override void OnInitialized()
-        {
-            NavigationManager.LocationChanged += (object sender, LocationChangedEventArgs e) =>
-            {
-                _ = JsInvokeAsync(JSInteropConstants.DestroyAllDialog);
-            };
-        }
-
-
        private bool _hasRendered = false;

        /// <summary>
--- a/components/modal/modalDialog/Modal.razor.cs
+++ b/components/modal/modalDialog/Modal.razor.cs
@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.ComponentModel;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Components;
+using Microsoft.AspNetCore.Components.Routing;
 using Microsoft.AspNetCore.Components.Web;
 using OneOf;

@ -13,6 +14,9 @@ namespace AntDesign
    /// </summary>
    public partial class Modal
    {
+        [Inject]
+        private NavigationManager NavigationManager { get; set; }
+
        #region Parameter

        /// <summary>
@ -151,7 +155,6 @@ namespace AntDesign
        [Parameter]
        public bool Visible { get; set; }

-
        /// <summary>
        /// Specify a function invoke when the modal dialog is visible or not
        /// </summary>
@ -366,6 +369,8 @@ namespace AntDesign

        private bool _hasFocus = false;

+        private bool _firstShow = true;
+
        private async Task OnAfterDialogShow()
        {
            if (!_hasFocus)
@ -377,6 +382,32 @@ namespace AntDesign
                    await ModalRef.OnOpen();
                }
            }
+
+            if (_firstShow)
+            {
+                _firstShow = false;
+                NavigationManager.LocationChanged += OnLocationChanged;
+            }
+        }
+
+        private void OnLocationChanged(object sender, LocationChangedEventArgs e)
+        {
+            // Modal create by Service
+            if (ModalRef != null)
+            {
+                return;
+            }
+            // Modal has been destroyed
+            if (!Visible && DestroyOnClose)
+            {
+                return;
+            }
+
+            if (_dialogWrapper.Dialog != null)
+            {
+                _ = JsInvokeAsync(JSInteropConstants.DelElementFrom, "#" + _dialogWrapper.Dialog.Id, GetContainer);
+                NavigationManager.LocationChanged -= OnLocationChanged;
+            }
        }

        private async Task OnAfterHide()
@ -390,6 +421,7 @@ namespace AntDesign

        private async Task OnBeforeDialogWrapperDestroy()
        {
+            NavigationManager.LocationChanged -= OnLocationChanged;
            await InvokeAsync(StateHasChanged);
        }

--- a/components/modal/modalDialog/ModalContainer.razor.cs
+++ b/components/modal/modalDialog/ModalContainer.razor.cs
@ -11,6 +11,9 @@ namespace AntDesign
        [Inject]
        private ModalService ModalService { get; set; }

+        [Inject]
+        private NavigationManager NavigationManager { get; set; }
+
        private readonly List<ModalRef> _modalRefs = new List<ModalRef>();

        protected override void OnInitialized()
@ -18,6 +21,14 @@ namespace AntDesign
            ModalService.OnModalOpenEvent += ModalService_OnModalOpenEvent;
            ModalService.OnModalCloseEvent += ModalService_OnModalCloseEvent;
            ModalService.OnModalUpdateEvent += ModalService_OnModalUpdateEvent;
+
+            NavigationManager.LocationChanged += OnLocationChanged;
+        }
+
+        private void OnLocationChanged(object sender, EventArgs e)
+        {
+            _modalRefs.Clear();
+            InvokeStateHasChanged();
        }

        private async Task ModalService_OnModalOpenEvent(ModalRef modalRef)
@ -56,6 +67,7 @@ namespace AntDesign
            ModalService.OnModalOpenEvent -= ModalService_OnModalOpenEvent;
            ModalService.OnModalCloseEvent -= ModalService_OnModalCloseEvent;
            ModalService.OnModalUpdateEvent -= ModalService_OnModalUpdateEvent;
+            NavigationManager.LocationChanged -= OnLocationChanged;

            base.Dispose(disposing);
        }
--- a/package.json
+++ b/package.json
@ -27,7 +27,8 @@
    "build:doc": "gulp build:preview --max-old-space-size=81920",
    "preinstall": "dotnet tool restore",
    "changelog": "node ./scripts/print-changelog",
-    "lint": "eslint ./components --ext .ts"
+    "lint": "eslint --ext .ts ./components",
+    "lint-fix": "eslint --fix --ext .ts ./components"
  },
  "devDependencies": {
    "@ant-design/colors": "^6.0.0",