Skip to content

Fix api collection icon rendering #1343

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix api collection icon rendering #1343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58d32f9
Add test for API Collection icon rendering in external references
RalucaP Nov 12, 2025
b385a24
Fix API Collection icon rendering for external references
RalucaP Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/SwiftDocC/Infrastructure/Link Resolution/ExternalPathHierarchyResolver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ extension LinkDestinationSummary {

/// Create a topic render render reference for this link summary and its content variants.
func makeTopicRenderReference() -> TopicRenderReference {
let (kind, role) = DocumentationContentRenderer.renderKindAndRole(kind, semantic: nil)
let (kind, role) = DocumentationContentRenderer.renderKindAndRole(kind, semantic: nil, linkSummary: self)
Copy link
Contributor

@patshaughnessy patshaughnessy Nov 13, 2025

Choose a reason for hiding this comment

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

Suggested change
let (kind, role) = DocumentationContentRenderer.renderKindAndRole(kind, semantic: nil, linkSummary: self)
let (kind, role) = DocumentationContentRenderer.renderKindAndRole(kind, linkSummary: self)

You set the default value for semantic to be nil, so there's no need to pass that in here now.

Copy link
Contributor

@d-ronnqvist d-ronnqvist Nov 13, 2025

Choose a reason for hiding this comment

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

Question: Is there some other value about the link summary that we can use to determine if it's an API collection?

Copy link
Contributor Author

@RalucaP RalucaP Nov 13, 2025

Choose a reason for hiding this comment

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

After analyzing the LinkDestinationSummary structure and the actual Apple documentation data, I found that there are no reliable alternatives to taskGroups for identifying API Collections in external references. The available fields (title patterns, URL patterns, abstract content, references) would all be fragile heuristics.
We could add an optional role to LinkDestinationSummary, but that would be a much larger change. Would you prefer that?

Copy link
Contributor

@d-ronnqvist d-ronnqvist Nov 13, 2025

Choose a reason for hiding this comment

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

The taskGroup information isn't a reliable source for this information. It's not requited for documentation sources to include it and we are also talking about removing it because the link summary isn't meant to be a representation of the documentation hierarchy.

Copy link
Contributor

@d-ronnqvist d-ronnqvist Nov 13, 2025

Choose a reason for hiding this comment

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

Is there another DocumentationNode.Kind value that API collection pages should be using? I see that we have both collection and collectionGroup but I don't recall what either of those are meant to represent. If not, should we introduce a dedicated "API Collection" kind and use that?

Copy link
Contributor

@patshaughnessy patshaughnessy Nov 13, 2025

Choose a reason for hiding this comment

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

I believe collection is the root/framework page, and collectionGroup are API Collections or pages that groups symbol pages together.


var titleVariants = VariantCollection(defaultValue: title)
var abstractVariants = VariantCollection(defaultValue: abstract ?? [])
Expand Down
5 changes: 4 additions & 1 deletion Sources/SwiftDocC/Model/Rendering/DocumentationContentRenderer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public class DocumentationContentRenderer {
return true
}

static func renderKindAndRole(_ kind: DocumentationNode.Kind?, semantic: Semantic?) -> (RenderNode.Kind, String) {
static func renderKindAndRole(_ kind: DocumentationNode.Kind?, semantic: Semantic? = nil, linkSummary: LinkDestinationSummary? = nil) -> (RenderNode.Kind, String) {
Copy link
Contributor

@d-ronnqvist d-ronnqvist Nov 13, 2025

Choose a reason for hiding this comment

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

If I understand this correctly, this method is not expected to receive both a semantic and a link summary because one represents a local page and the other represents an external page. If we need a version that accepts a link summary we could consider having either a separate overload (that calls into a private common implementation) or a parameter that can be either a Semantic or a LinkDestinationSummary.

Copy link
Contributor

@sofiaromorales sofiaromorales Nov 13, 2025

Choose a reason for hiding this comment

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

If it's not expected to pass both semantic and linkSummary at the same invocation call, let's split the method into two making them an overload.

The original

 static func renderKindAndRole(_ kind: DocumentationNode.Kind?, semantic: Semantic?) -> (RenderNode.Kind, String) {

and the overload

 static func renderKindAndRole(_ kind: DocumentationNode.Kind?, linkSummary: LinkDestinationSummary?) -> (RenderNode.Kind, String) {

Then the common logic can be encapsulated in another function.

guard let kind else {
return (.article, role(for: .article).rawValue)
}
Expand All @@ -270,6 +270,9 @@ public class DocumentationContentRenderer {
default:
if let article = semantic as? Article {
return (.article, roleForArticle(article, nodeKind: kind).rawValue)
} else if kind == .article, let summary = linkSummary, summary.taskGroups != nil {
Copy link
Contributor

@d-ronnqvist d-ronnqvist Nov 13, 2025

Choose a reason for hiding this comment

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

The task groups of an link summary isn't a value that external documentation sources have to provide—and DocC doesn't include it in its link summaries unless specifically configured to do so—so it can't be reliably used to determine if an external page is an API collection or not.

// For external references: Articles with task groups are API Collections and should have collectionGroup role
return (.article, Self.role(for: .collectionGroup).rawValue)
} else {
return (.article, role)
}
Expand Down
18 changes: 18 additions & 0 deletions Tests/SwiftDocCTests/Rendering/DocumentationContentRendererTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,24 @@ class DocumentationContentRendererTests: XCTestCase {
]
)
}

func testRenderKindAndRoleForAPICollection() throws {
Copy link
Contributor

@patshaughnessy patshaughnessy Nov 13, 2025

Choose a reason for hiding this comment

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

Test looks good, but you might want to add another test for the case when taskGroups is nil, i.e. test that this continues to work for references to a normal article.

And what should happen if taskGroups is an empty array?

// Articles with task groups should get collectionGroup role for external references
let apiCollectionSummary = LinkDestinationSummary(
kind: .article,
language: .swift,
relativePresentationURL: URL(string: "/documentation/test/external/api-collection")!,
referenceURL: URL(string: "doc://com.example.test/documentation/test/external/api-collection")!,
title: "External API Collection",
availableLanguages: [.swift],
taskGroups: [LinkDestinationSummary.TaskGroup(title: "Symbols", identifiers: ["doc://com.example.test/symbol1"])],
variants: []
)

let (kind, role) = DocumentationContentRenderer.renderKindAndRole(.article, semantic: nil, linkSummary: apiCollectionSummary)
XCTAssertEqual(kind, RenderNode.Kind.article)
XCTAssertEqual(role, "collectionGroup")
}
}

private extension DocumentationDataVariantsTrait {
Expand Down