From bfb9e3a56f40e3b11518560252de8ac75f52d68a Mon Sep 17 00:00:00 2001 From: zxyao Date: Wed, 28 Feb 2024 23:29:48 +0800 Subject: [PATCH] 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 --- components/.eslintrc | 7 +- .../JsInterop/modules/components/overlay.ts | 178 +++++++++--------- .../modules/dom/manipulationHelper.ts | 102 +++++----- components/drawer/DrawerContainer.razor.cs | 12 ++ .../confirmDialog/ComfirmContainer.razor.cs | 15 +- components/modal/core/Dialog.razor.cs | 12 -- components/modal/modalDialog/Modal.razor.cs | 34 +++- .../modal/modalDialog/ModalContainer.razor.cs | 12 ++ package.json | 3 +- 9 files changed, 227 insertions(+), 148 deletions(-) diff --git a/components/.eslintrc b/components/.eslintrc index d33c4caf..bd09d03f 100644 --- a/components/.eslintrc +++ b/components/.eslintrc @@ -14,7 +14,12 @@ "indent": [ "error", 2, - { "ignoreComments": true } + { + "ignoreComments": true, + "SwitchCase": 1, + "flatTernaryExpressions": false, + "offsetTernaryExpressions": false + } ] } } \ No newline at end of file diff --git a/components/core/JsInterop/modules/components/overlay.ts b/components/core/JsInterop/modules/components/overlay.ts index 5d729546..b7a46887 100644 --- 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.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; - }; - 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; - }; + 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, + }; + 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) + }; + 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, - }; - 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) - }; - return position; - }; + 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; + }; + 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; + }; } } //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.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.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 + }; + 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) - }; - 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.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; }; - position.left = Overlay.reversePositionValue(position.right, container.scrollWidth, overlayWidth) - 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 + }; + position.left = Overlay.reversePositionValue(position.right, container.scrollWidth, overlayWidth) + return position; + }; } } //fallback - should not happen, but to avoid crashing scenarios, revert to BottomLeft diff --git a/components/core/JsInterop/modules/dom/manipulationHelper.ts b/components/core/JsInterop/modules/dom/manipulationHelper.ts index 70b2086d..e9ca2a01 100644 --- 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(); @@ -45,7 +44,7 @@ export class manipulationHelper { } } } - + static copyElement(element) { if (!this.copyElementAsRichText(element)) { this.copy(element.innerText); @@ -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) { @@ -144,10 +146,14 @@ export class manipulationHelper { parentElement.scrollTop = element.offsetTop; } else if (element && element instanceof HTMLElement) { element.scrollIntoView({ behavior: 'smooth', block: 'nearest', inline: 'start' }); - } - } + } + } - 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() : {}; - + const oldBodyCache = state.oldBodyCacheStack.length > 0 ? state.oldBodyCacheStack.pop() : {}; styleHelper.css(document.body, { "position": oldBodyCache["position"] ?? null, @@ -244,16 +258,18 @@ 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 * source https://github.com/react-component/util/blob/master/src/getScrollBarSize.tsx - * + * * @param fresh force get scrollBar size and don't use cache - * @returns + * @returns */ static getScrollBarSize = (fresh: boolean = false) => { if (typeof document === "undefined") { @@ -294,4 +310,4 @@ export class manipulationHelper { } return cachedScrollBarSize; }; -} \ No newline at end of file +} diff --git a/components/drawer/DrawerContainer.razor.cs b/components/drawer/DrawerContainer.razor.cs index 694b24e1..968d63ce 100644 --- 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); } diff --git a/components/modal/confirmDialog/ComfirmContainer.razor.cs b/components/modal/confirmDialog/ComfirmContainer.razor.cs index c707443f..0ad00576 100644 --- 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 _confirmRefs = new List(); #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(); + } + /// /// create and open a Confirm dialog /// @@ -116,6 +128,7 @@ namespace AntDesign ModalService.OnConfirmUpdateEvent -= OnConfirmUpdate; ConfirmService.OnOpenEvent -= OnConfirmOpen; + NavigationManager.LocationChanged -= OnLocationChanged; base.Dispose(disposing); } diff --git a/components/modal/core/Dialog.razor.cs b/components/modal/core/Dialog.razor.cs index 8fcd70cb..34717148 100644 --- 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; /// diff --git a/components/modal/modalDialog/Modal.razor.cs b/components/modal/modalDialog/Modal.razor.cs index 8abe9223..0b5b5a04 100644 --- 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 /// public partial class Modal { + [Inject] + private NavigationManager NavigationManager { get; set; } + #region Parameter /// @@ -151,7 +155,6 @@ namespace AntDesign [Parameter] public bool Visible { get; set; } - /// /// Specify a function invoke when the modal dialog is visible or not /// @@ -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); } diff --git a/components/modal/modalDialog/ModalContainer.razor.cs b/components/modal/modalDialog/ModalContainer.razor.cs index 31268689..a2aab184 100644 --- 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 _modalRefs = new List(); 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); } diff --git a/package.json b/package.json index c0ca6fe2..7d7df3ff 100644 --- 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",