Skip to content

Document URLs Data Resolver #19316

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

Merged
merged 20 commits into from
May 19, 2025

Conversation

madsrasmussen
Copy link
Contributor

@madsrasmussen madsrasmussen commented May 13, 2025

Fixes #18317

This PR updates the Multi URL Picker UI to render variant-aware URLs and adds the variant name to the URL title field. It introduces a DocumentURLsDataResolver to obtain the variant-aware URLs. This resolver is now reused for both the workspace URLs in the info app and the Multi URL Picker.

The workspace URLs now display only the URLs for the active variant unless the document is invariant; in that case, it will show all URLs.

Updates to the Multi URL Picker
https://github.com/user-attachments/assets/f6f8a7b7-f747-4aad-8250-c2edfeb175ac

Updates to the document URLs app
https://github.com/user-attachments/assets/f3fc25ee-8ee4-42e8-8459-a6272dcd67fd

@madsrasmussen madsrasmussen marked this pull request as ready for review May 14, 2025 18:08
Copy link
Contributor

@Copilot Copilot AI left a 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 updates the Multi URL Picker and document URLs app to support variant-aware URL resolution by introducing and integrating a new DocumentURLsDataResolver. Key changes include:

  • Wrapping content in the workspace info app with a new container element for styling.
  • Updating the link picker modal and multi URL picker components to use new repository and data resolver methods for fetching document and media URLs.
  • Adjusting document URL models and integrating the DocumentURLsDataResolver in the document links workspace info app.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Umbraco.Web.UI.Client/src/packages/relations/relations/reference/workspace-info-app/entity-references-workspace-view-info.element.ts Added a container div for layout consistency.
src/Umbraco.Web.UI.Client/src/packages/multi-url-picker/link-picker-modal/link-picker-modal.element.ts Replaced legacy document/media repositories with new repository and data resolver usage.
src/Umbraco.Web.UI.Client/src/packages/multi-url-picker/components/input-multi-url/input-multi-url.element.ts Integrated asynchronous URL resolution for each link and added resolved URL state.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/url/repository/types.ts Updated the culture property from optional to required (accepting null values).
src/Umbraco.Web.UI.Client/src/packages/documents/documents/url/info-app/document-links-workspace-info-app.element.ts Adjusted link rendering logic and integrated the DocumentURLsDataResolver.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/url/index.ts Exported the new DocumentURLsDataResolver.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/url/document-urls-data-resolver.ts Added a new resolver for obtaining variant-aware document URLs.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/item/document-item-data-resolver.ts Minor update with a TODO regarding variant context usage.
src/Umbraco.Web.UI.Client/src/packages/core/workspace/info-app/global-components/workspace-info-app-layout.element.ts Removed the custom container padding style.
Comments suppressed due to low confidence (1)

src/Umbraco.Web.UI.Client/src/packages/documents/documents/url/repository/types.ts:7

  • [nitpick] Changing the 'culture' property to be non-optional may require verifying that all consumers of UmbDocumentUrlModel supply a defined value (even if null) instead of undefined.
culture: string | null;

@madsrasmussen
Copy link
Contributor Author

@AndyButland, you have recently made some UI updates to the Multi URL Picker. Can you help ensure that the feature still works the way it should?

Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@madsrasmussen - sure, I've tested it out locally. Mostly seems to work just fine, and I can see the new multi-lingual aware behaviour is functioning nicely, but I found these small things:

  • If you select a document, or media, remove it again and switch to the "Manual" tab the previously selected document or media's URL is still displayed. Given I've clicked to remove it and confirmed it via the warning, I think it should be empty at this point.
  • Also, having followed the same process, it looks like the "title" isn't reset, as once I've provided a manual URL and closed the picker, the previously selected document or media's title is shown in the document workspace.
  • If you clear the title, you get a validation message, but it appears under the anchor tag.
    • This may have been my doing with a previous update - I changed the behaviour in this PR so we checked for the existence of a title rather than a URL, as for unpublished pages there is no URL.
    • Not sure what's best here - could be that we restore that this isn't mandatory and it's enough to just choose or enter the link (so we need to check for either a unique or a manually entered URL to determine if a link has been correctly populated via the picker).
    • Or we keep it mandatory - it is automatically filled for all three modes when a URL is provided - but the validation message should move so it indicates the whole form is invalid rather than one field, and the error message tweaked.

Copy link
Member

@nielslyngsoe nielslyngsoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, but we need to update with Variant Context

@nielslyngsoe nielslyngsoe merged commit 930a29f into release/16.0 May 19, 2025
21 of 23 checks passed
@nielslyngsoe nielslyngsoe deleted the v16/hotfix/document-urls-data-resolver branch May 19, 2025 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants