Inactivate known external links when needed #873
Conversation
When multiple targets that reference each other are compiled together using the experimental DocC support, this array will indicate which targets are available for linking purposes. This allows the renderer to inactivate links in the situation where a single target is being presented on its own and therefore cannot link to the other target.
There was a problem hiding this comment.
- Do we plan to have a separate PR for known external asset references? Or is that not a concern?
- We use the
Referencecomponent in couple of other places, like tutorials, button link, cards, mentioned in, relationships, etc. Do we also need to make this change there? or is it only a concern for links in the primary content and declarations? - I tried my best to understand why this is needed, but I obviously don't have all the context. Maybe this won't work for obvious reasons I'm not aware of. But have you considered maybe removing items from the references object if their module name is not included in the
includedArchiveIdentifierslist? Then we just let existing logic in each component to take over.
| const { includedArchiveIdentifiers = [] } = context; | ||
| return !!(includedArchiveIdentifiers.length && ( | ||
| !includedArchiveIdentifiers.some(id => ( | ||
| identifier?.startsWith(`doc://${id}`) |
There was a problem hiding this comment.
- Do we need to worry about capitalization here?
- I think we need to check if the identifier starts with
doc://${id}/documentation. Just to make sure that the id matches exactly. (For example, Foo and FooBar)
There was a problem hiding this comment.
- Capitalization would be matched exactly as-is here. I don't think we would want to be less strict since we're given the exact identifier.
- That would only work for documentation pages and not tutorial pages. We could make this more strict, but I think checking for the special scheme and bundle ID should be sufficient (@d-ronnqvist correct me if I'm wrong)
I don't believe that's a concern but @d-ronnqvist let me know if I'm mistaken.
The
I haven't tried this, but this could potentially be a better approach, although it might be difficult to find the right layer to add this logic. I can look into it. |
What about components that use
I haven't thought too deeply into this, but maybe when we set references in the |
I think the |
|
Closing in favor of #878 |
Bug/issue #, if applicable: 118834404
Summary
This adds logic to inactivate links to pages from other targets unless they have been included in the same combined archive.
Dependencies
swiftlang/swift-docc-plugin#84
Testing
You can use the archives from this fixture for the convenience of testing purposes:
archives.zip
Steps:
VUE_APP_DEV_SERVER_PROXY=/path/to/Combined\ ExternalLinks\ Documentation.doccarchive npm run serveand open http://localhost:8080/documentation/outer/outerclass/dosomething(with:)VUE_APP_DEV_SERVER_PROXY=/path/to/Outer.doccarchive npm run serveand open http://localhost:8080/documentation/outer/outerclass/dosomething(with:)Known issues:
Due to the way that the Render JSON and Navigator JSON are loaded independently by the renderer, it's possible that the links in step 4 may be initially linked for a very brief moment until the navigator has finished loading. I've discussed this with @d-ronnqvist, and this limitation is acceptable for now given the way that things are structured.
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
npm test, and it succeeded