Skip to content

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

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 2 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions Sources/SwiftDocC/Model/Rendering/RenderNodeTranslator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
))
}
Expand Down
27 changes: 24 additions & 3 deletions Sources/SwiftDocC/Semantics/Metadata/CallToAction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

should this internally call defaultLabel(for: nil)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea: c83c4a3.

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"
}
}
}
}
8 changes: 7 additions & 1 deletion Sources/docc/DocCDocumentation.docc/DocC.symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
100 changes: 40 additions & 60 deletions Tests/SwiftDocCTests/Rendering/SampleDownloadTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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)
}
}
46 changes: 30 additions & 16 deletions Tests/SwiftDocCTests/Semantics/CallToActionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
}
}
Original file line number Diff line number Diff line change
@@ -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. -->
Original file line number Diff line number Diff line change
@@ -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. -->