Label Sample-Code Call-to-Action buttons with "View Source" (swiftlang#566)

ethan-kusters · ethan-kusters · commit aec509c4a26b · 2023-04-26T11:40:55.000-07:00
Updates the logic for `@CallToAction` buttons that are attached
to `sampleCode` pages and use the `link` purpose to render with
a more specific “View Source” title instead of the more generic
“Visit” title.

This makes the intent of the button more clear.

Resolves rdar://108013602
diff --git a/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift b/Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift
@@ -795,7 +795,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
                     action: .reference(
                         identifier: downloadIdentifier,
                         isActive: true,
-                        overridingTitle: callToAction.buttonLabel,
+                        overridingTitle: callToAction.buttonLabel(for: article.metadata?.pageKind?.kind),
                         overridingTitleInlineContent: nil))
                 externalLocationReferences[url.description] = ExternalLocationReference(identifier: downloadIdentifier)
             } else if let fileReference = callToAction.file,
@@ -804,7 +804,7 @@ public struct RenderNodeTranslator: SemanticVisitor {
                 node.sampleDownload = .init(action: .reference(
                     identifier: downloadIdentifier,
                     isActive: true,
-                    overridingTitle: callToAction.buttonLabel,
+                    overridingTitle: callToAction.buttonLabel(for: article.metadata?.pageKind?.kind),
                     overridingTitleInlineContent: nil
                 ))
             }
diff --git a/Sources/SwiftDocC/Semantics/Metadata/CallToAction.swift b/Sources/SwiftDocC/Semantics/Metadata/CallToAction.swift
@@ -25,7 +25,9 @@ import Markdown
 /// The link text can also be specified in one of two ways:
 /// - The `purpose` parameter can be used to use a default button label. There are two valid values:
 ///   - `download` indicates that the link is to a downloadable file. The button will be labeled "Download".
-///   - `link` indicates that the link is to an external webpage. The button will be labeled "Visit".
+///   - `link` indicates that the link is to an external webpage.
+///
+///      The button will be labeled "Visit" when used on article pages and "View Source" when used on sample code pages.
 /// - The `label` parameter specifies the literal text to use as the button label.
 ///
 /// `@CallToAction` requires one of `url` or `path`, and one of `purpose` or `label`. Specifying both
@@ -79,11 +81,20 @@ public final class CallToAction: Semantic, AutomaticDirectiveConvertible {
 
     /// The computed label for this Call to Action, whether provided directly via ``label`` or
     /// indirectly via ``purpose``.
+    @available(*, deprecated, renamed: "buttonLabel(for:)")
     public var buttonLabel: String {
+        return buttonLabel(for: nil)
+    }
+    
+    /// The label that should be used when rendering the user-interface for this call to action button.
+    ///
+    /// This can be provided directly via the ``label`` parameter or indirectly via the given ``purpose`` and
+    /// associated page kind.
+    public func buttonLabel(for pageKind: Metadata.PageKind.Kind?) -> String {
         if let label = label {
             return label
         } else if let purpose = purpose {
-            return purpose.defaultLabel
+            return purpose.defaultLabel(for: pageKind)
         } else {
             // The `validate()` method ensures that this type should never be constructed without
             // one of the above.
@@ -178,12 +189,22 @@ extension CallToAction {
 extension CallToAction.Purpose {
     /// The label that will be applied to a Call to Action with this purpose if it doesn't provide
     /// a separate label.
+    @available(*, deprecated, message: "Replaced with 'CallToAction.buttonLabel(for:)'.")
     public var defaultLabel: String {
+        return defaultLabel(for: nil)
+    }
+    
+    fileprivate func defaultLabel(for pageKind: Metadata.PageKind.Kind?) -> String {
         switch self {
         case .download:
             return "Download"
         case .link:
-            return "Visit"
+            switch pageKind {
+            case .article, .none:
+                return "Visit"
+            case .sampleCode:
+                return "View Source"
+            }
         }
     }
 }
diff --git a/Sources/docc/DocCDocumentation.docc/DocC.symbols.json b/Sources/docc/DocCDocumentation.docc/DocC.symbols.json
@@ -817,7 +817,13 @@
             "text" : "  - `download` indicates that the link is to a downloadable file. The button will be labeled \"Download\"."
           },
           {
-            "text" : "  - `link` indicates that the link is to an external webpage. The button will be labeled \"Visit\"."
+            "text" : "  - `link` indicates that the link is to an external webpage."
+          },
+          {
+            "text" : ""
+          },
+          {
+            "text" : "     The button will be labeled \"Visit\" when used on article pages and \"View Source\" when used on sample code pages."
           },
           {
             "text" : "- The `label` parameter specifies the literal text to use as the button label."
diff --git a/Tests/SwiftDocCTests/Rendering/SampleDownloadTests.swift b/Tests/SwiftDocCTests/Rendering/SampleDownloadTests.swift
@@ -75,20 +75,8 @@ class SampleDownloadTests: XCTestCase {
     }
 
     func testParseSampleDownload() throws {
-        let (bundle, context) = try testBundleAndContext(named: "SampleBundle")
-        let reference = ResolvedTopicReference(
-            bundleIdentifier: bundle.identifier,
-            path: "/documentation/SampleBundle/MySample",
-            sourceLanguage: .swift
-        )
-        let article = try XCTUnwrap(context.entity(with: reference).semantic as? Article)
-        var translator = RenderNodeTranslator(
-            context: context,
-            bundle: bundle,
-            identifier: reference,
-            source: nil
-        )
-        let renderNode = try XCTUnwrap(translator.visitArticle(article) as? RenderNode)
+        let renderNode = try renderNodeFromSampleBundle(at: "/documentation/SampleBundle/MySample")
+        
         let sampleCodeDownload = try XCTUnwrap(renderNode.sampleDownload)
         guard case .reference(identifier: let ident, isActive: true, overridingTitle: "Download", overridingTitleInlineContent: nil) = sampleCodeDownload.action else {
             XCTFail("Unexpected action in callToAction")
@@ -98,20 +86,8 @@ class SampleDownloadTests: XCTestCase {
     }
 
     func testParseSampleLocalDownload() throws {
-        let (bundle, context) = try testBundleAndContext(named: "SampleBundle")
-        let reference = ResolvedTopicReference(
-            bundleIdentifier: bundle.identifier,
-            path: "/documentation/SampleBundle/MyLocalSample",
-            sourceLanguage: .swift
-        )
-        let article = try XCTUnwrap(context.entity(with: reference).semantic as? Article)
-        var translator = RenderNodeTranslator(
-            context: context,
-            bundle: bundle,
-            identifier: reference,
-            source: nil
-        )
-        let renderNode = try XCTUnwrap(translator.visitArticle(article) as? RenderNode)
+        let renderNode = try renderNodeFromSampleBundle(at: "/documentation/SampleBundle/MyLocalSample")
+        
         let sampleCodeDownload = try XCTUnwrap(renderNode.sampleDownload)
         guard case .reference(identifier: let ident, isActive: true, overridingTitle: "Download", overridingTitleInlineContent: nil) = sampleCodeDownload.action else {
             XCTFail("Unexpected action in callToAction")
@@ -121,20 +97,7 @@ class SampleDownloadTests: XCTestCase {
     }
 
     func testSampleDownloadRoundtrip() throws {
-        let (bundle, context) = try testBundleAndContext(named: "SampleBundle")
-        let reference = ResolvedTopicReference(
-            bundleIdentifier: bundle.identifier,
-            path: "/documentation/SampleBundle/MySample",
-            sourceLanguage: .swift
-        )
-        let article = try XCTUnwrap(context.entity(with: reference).semantic as? Article)
-        var translator = RenderNodeTranslator(
-            context: context,
-            bundle: bundle,
-            identifier: reference,
-            source: nil
-        )
-        let renderNode = try XCTUnwrap(translator.visitArticle(article) as? RenderNode)
+        let renderNode = try renderNodeFromSampleBundle(at: "/documentation/SampleBundle/MySample")
 
         let encoder = JSONEncoder()
         let decoder = JSONDecoder()
@@ -161,12 +124,12 @@ class SampleDownloadTests: XCTestCase {
 
         XCTAssertEqual(origIdent, decodedIdent)
     }
-
-    func testSampleDownloadRelativeURL() throws {
+    
+    private func renderNodeFromSampleBundle(at referencePath: String) throws -> RenderNode {
         let (bundle, context) = try testBundleAndContext(named: "SampleBundle")
         let reference = ResolvedTopicReference(
             bundleIdentifier: bundle.identifier,
-            path: "/documentation/SampleBundle/RelativeURLSample",
+            path: referencePath,
             sourceLanguage: .swift
         )
         let article = try XCTUnwrap(context.entity(with: reference).semantic as? Article)
@@ -176,7 +139,11 @@ class SampleDownloadTests: XCTestCase {
             identifier: reference,
             source: nil
         )
-        let renderNode = try XCTUnwrap(translator.visitArticle(article) as? RenderNode)
+        return try XCTUnwrap(translator.visitArticle(article) as? RenderNode)
+    }
+
+    func testSampleDownloadRelativeURL() throws {
+        let renderNode = try renderNodeFromSampleBundle(at: "/documentation/SampleBundle/RelativeURLSample")
         let sampleCodeDownload = try XCTUnwrap(renderNode.sampleDownload)
         guard case .reference(identifier: let ident, isActive: true, overridingTitle: "Download", overridingTitleInlineContent: nil) = sampleCodeDownload.action else {
             XCTFail("Unexpected action in callToAction")
@@ -197,20 +164,7 @@ class SampleDownloadTests: XCTestCase {
     }
 
     func testExternalLocationRoundtrip() throws {
-        let (bundle, context) = try testBundleAndContext(named: "SampleBundle")
-        let reference = ResolvedTopicReference(
-            bundleIdentifier: bundle.identifier,
-            path: "/documentation/SampleBundle/RelativeURLSample",
-            sourceLanguage: .swift
-        )
-        let article = try XCTUnwrap(context.entity(with: reference).semantic as? Article)
-        var translator = RenderNodeTranslator(
-            context: context,
-            bundle: bundle,
-            identifier: reference,
-            source: nil
-        )
-        let renderNode = try XCTUnwrap(translator.visitArticle(article) as? RenderNode)
+        let renderNode = try renderNodeFromSampleBundle(at: "/documentation/SampleBundle/RelativeURLSample")
         let sampleCodeDownload = try XCTUnwrap(renderNode.sampleDownload)
         guard case .reference(identifier: let ident, isActive: true, overridingTitle: "Download", overridingTitleInlineContent: nil) = sampleCodeDownload.action else {
             XCTFail("Unexpected action in callToAction")
@@ -260,4 +214,30 @@ class SampleDownloadTests: XCTestCase {
             XCTAssertEqual(firstJson, finalJson)
         }
     }
+    
+    func testExternalLinkOnSampleCodePage() throws {
+        let renderNode = try renderNodeFromSampleBundle(at: "/documentation/SampleBundle/MyExternalSample")
+        let sampleCodeDownload = try XCTUnwrap(renderNode.sampleDownload)
+        guard case .reference(identifier: let identifier, isActive: true, overridingTitle: "View Source", overridingTitleInlineContent: nil) = sampleCodeDownload.action else {
+            XCTFail("Unexpected action in callToAction")
+            return
+        }
+        
+        XCTAssertEqual(identifier.identifier, "https://www.example.com/source-repository.git")
+        let reference = try XCTUnwrap(renderNode.references[identifier.identifier])
+        XCTAssert(reference is ExternalLocationReference)
+    }
+    
+    func testExternalLinkOnRegularArticlePage() throws {
+        let renderNode = try renderNodeFromSampleBundle(at: "/documentation/SampleBundle/MyArticle")
+        let sampleCodeDownload = try XCTUnwrap(renderNode.sampleDownload)
+        guard case .reference(identifier: let identifier, isActive: true, overridingTitle: "Visit", overridingTitleInlineContent: nil) = sampleCodeDownload.action else {
+            XCTFail("Unexpected action in callToAction")
+            return
+        }
+        
+        XCTAssertEqual(identifier.identifier, "https://www.example.com")
+        let reference = try XCTUnwrap(renderNode.references[identifier.identifier])
+        XCTAssert(reference is ExternalLocationReference)
+    }
 }
diff --git a/Tests/SwiftDocCTests/Semantics/CallToActionTests.swift b/Tests/SwiftDocCTests/Semantics/CallToActionTests.swift
@@ -137,33 +137,47 @@ class CallToActionTests: XCTestCase {
     }
 
     func testDefaultLabel() throws {
-        func assertExpectedLabel(source: String, expectedLabel: String) throws {
+        func assertExpectedLabel(source: String, expectedDefaultLabel: String, expectedSampleCodeLabel: String) throws {
             let document = Document(parsing: source, options: .parseBlockDirectives)
-            let directive = document.child(at: 0) as? BlockDirective
-            XCTAssertNotNil(directive)
+            let directive = try XCTUnwrap(document.child(at: 0) as? BlockDirective)
 
             let (bundle, context) = try testBundleAndContext(named: "SampleBundle")
 
-            directive.map { directive in
-                var problems = [Problem]()
-                XCTAssertEqual(CallToAction.directiveName, directive.name)
-                let callToAction = CallToAction(from: directive, source: nil, for: bundle, in: context, problems: &problems)
-                XCTAssertNotNil(callToAction)
-                XCTAssert(problems.isEmpty)
-                XCTAssertEqual(callToAction?.buttonLabel, expectedLabel)
-            }
+            var problems = [Problem]()
+            XCTAssertEqual(CallToAction.directiveName, directive.name)
+            let callToAction = try XCTUnwrap(CallToAction(from: directive, source: nil, for: bundle, in: context, problems: &problems))
+            XCTAssert(problems.isEmpty)
+            
+            XCTAssertEqual(callToAction.buttonLabel(for: nil), expectedDefaultLabel)
+            XCTAssertEqual(callToAction.buttonLabel(for: .article), expectedDefaultLabel)
+            XCTAssertEqual(callToAction.buttonLabel(for: .sampleCode), expectedSampleCodeLabel)
         }
 
-        var validLabels: [(arg: String, label: String)] = []
+        var validLabels: [(arg: String, defaultLabel: String, sampleCodeLabel: String)] = []
         for buttonKind in CallToAction.Purpose.allCases {
-            validLabels.append(("purpose: \(buttonKind)", buttonKind.defaultLabel))
+            let expectedDefaultLabel: String
+            let expectedSampleCodeLabel: String
+            switch buttonKind {
+            case .download:
+                expectedDefaultLabel = "Download"
+                expectedSampleCodeLabel = "Download"
+            case .link:
+                expectedDefaultLabel = "Visit"
+                expectedSampleCodeLabel = "View Source"
+            }
+            
+            validLabels.append(("purpose: \(buttonKind)", expectedDefaultLabel, expectedSampleCodeLabel))
             // Ensure that adding a label argument overrides the kind's default label
-            validLabels.append(("purpose: \(buttonKind), label: \"Button\"", "Button"))
+            validLabels.append(("purpose: \(buttonKind), label: \"Button\"", "Button", "Button"))
         }
 
-        for (arg, label) in validLabels {
+        for (arg, defaultLabel, sampleCodeLabel) in validLabels {
             let directive = "@CallToAction(file: \"Downloads/plus.svg\", \(arg))"
-            try assertExpectedLabel(source: directive, expectedLabel: label)
+            try assertExpectedLabel(
+                source: directive,
+                expectedDefaultLabel: defaultLabel,
+                expectedSampleCodeLabel: sampleCodeLabel
+            )
         }
     }
 }
diff --git a/Tests/SwiftDocCTests/Test Bundles/SampleBundle.docc/MyArticle.md b/Tests/SwiftDocCTests/Test Bundles/SampleBundle.docc/MyArticle.md
@@ -0,0 +1,9 @@
+# MyArticle
+
+@Metadata {
+    @CallToAction(url: "https://www.example.com", purpose: link)
+}
+
+Check out this cool website.
+
+<!-- Copyright (c) 2023 Apple Inc and the Swift Project authors. All Rights Reserved. -->
diff --git a/Tests/SwiftDocCTests/Test Bundles/SampleBundle.docc/MyExternalSample.md b/Tests/SwiftDocCTests/Test Bundles/SampleBundle.docc/MyExternalSample.md
@@ -0,0 +1,10 @@
+# MyExternalSample
+
+@Metadata {
+    @CallToAction(url: "https://www.example.com/source-repository.git", purpose: link)
+    @PageKind(sampleCode)
+}
+
+Check out my cool sample code project.
+
+<!-- Copyright (c) 2023 Apple Inc and the Swift Project authors. All Rights Reserved. -->
