-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Closing redirecting modals fix #19420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closing redirecting modals fix #19420
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses an issue where modal URLs persist after closing, which prevented users from reusing the same URL. Key changes include:
- Adding and refining destroy methods to ensure proper cleanup.
- Adjusting modal redirect logic by implementing a timeout mechanism.
- Handling potential initialization failures in repository functions.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/packages/core/workspace/submittable/submittable-workspace-context-base.ts | Added an override destroy method to clean up component state. |
src/packages/core/workspace/entity-detail/entity-detail-workspace-base.ts | Removed a duplicate call to set the "isNew" state. |
src/packages/core/workspace/controllers/workspace-is-new-redirect.controller.ts | Introduced a timeout to delay redirect and updated the destroy method, although a duplicate super.destroy() call is present. |
src/packages/core/router/router-slot/router-slot.ts | Added parent cleanup and improved redirection logic with an additional browser URL check. |
src/packages/core/repository/item/item-repository.interface.ts | Updated the return type for the items method to allow undefined. |
src/packages/core/repository/item/item-repository-base.ts | Wrapped initialization in a try-catch block, but an inconsistency in the return type was introduced. |
src/packages/content/content/workspace/content-detail-workspace-base.ts | Removed a redundant setIsNew(false) call to ensure proper state management. |
...eb.UI.Client/src/packages/core/workspace/controllers/workspace-is-new-redirect.controller.ts
Outdated
Show resolved
Hide resolved
...eb.UI.Client/src/packages/core/workspace/controllers/workspace-is-new-redirect.controller.ts
Outdated
Show resolved
Hide resolved
src/Umbraco.Web.UI.Client/src/packages/core/repository/item/item-repository-base.ts
Show resolved
Hide resolved
…ers/workspace-is-new-redirect.controller.ts Co-authored-by: Copilot <[email protected]>
…ers/workspace-is-new-redirect.controller.ts Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modals seem to work. Tested a few complex scenarios including blocks in an RTE on another tab, which generates quite a long URL.
Fixes issue with creating Documents from a Collection, on a real server, where 2. 3. or 4. time ends up the the modal URL stay despite the modal closing. which prevents the user from navigating to that exact URL again.
Please carefully test various deep modals etc. complex URL situations.