Skip to content
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

Add authoring support for @TabNavigator directive #379

Merged
merged 1 commit into from
Sep 26, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ extension RenderBlockContent: TextIndexing {
}.joined(separator: " ")
case .small(let small):
return small.inlineContent.rawIndexableTextContent(references: references)
case .tabNavigator(let tabNavigator):
return tabNavigator.tabs.map { tab in
return tab.content.rawIndexableTextContent(references: references)
}.joined(separator: " ")
default:
fatalError("unknown RenderBlockContent case in rawIndexableTextContent")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ public enum RenderBlockContent: Equatable {

/// A paragraph of small print content that should be rendered in a small font.
case small(Small)

/// A collection of content that should be rendered in a tab-based layout.
case tabNavigator(TabNavigator)

// Warning: If you add a new case to this enum, make sure to handle it in the Codable
// conformance at the bottom of this file, and in the `rawIndexableTextContent` method in
Expand Down Expand Up @@ -476,6 +479,21 @@ public enum RenderBlockContent: Equatable {
/// The inline content that should be rendered.
public let inlineContent: [RenderInlineContent]
}

/// A collection of content that should be rendered in a tab-based layout.
public struct TabNavigator: Codable, Equatable {
/// The tabs that make up this tab navigator.
public let tabs: [Tab]

/// A titled tab inside a tab-based layout container.
public struct Tab: Codable, Equatable {
/// The title that should be used to identify this tab.
public let title: String

/// The content that should be rendered in this tab.
public let content: [RenderBlockContent]
}
}
}

// Writing a manual Codable implementation for tables because the encoding of `extendedData` does
Expand Down Expand Up @@ -563,6 +581,7 @@ extension RenderBlockContent: Codable {
case request, response
case header, rows
case numberOfColumns, columns
case tabs
}

public init(from decoder: Decoder) throws {
Expand Down Expand Up @@ -616,11 +635,17 @@ extension RenderBlockContent: Codable {
self = try .small(
Small(inlineContent: container.decode([RenderInlineContent].self, forKey: .inlineContent))
)
case .tabNavigator:
self = try .tabNavigator(
TabNavigator(
tabs: container.decode([TabNavigator.Tab].self, forKey: .tabs)
)
)
}
}

private enum BlockType: String, Codable {
case paragraph, aside, codeListing, heading, orderedList, unorderedList, step, endpointExample, dictionaryExample, table, termList, row, small
case paragraph, aside, codeListing, heading, orderedList, unorderedList, step, endpointExample, dictionaryExample, table, termList, row, small, tabNavigator
}

private var type: BlockType {
Expand All @@ -638,6 +663,7 @@ extension RenderBlockContent: Codable {
case .termList: return .termList
case .row: return .row
case .small: return .small
case .tabNavigator: return .tabNavigator
default: fatalError("unknown RenderBlockContent case in type property")
}
}
Expand Down Expand Up @@ -688,6 +714,8 @@ extension RenderBlockContent: Codable {
try container.encode(row.columns, forKey: .columns)
case .small(let small):
try container.encode(small.inlineContent, forKey: .inlineContent)
case .tabNavigator(let tabNavigator):
try container.encode(tabNavigator.tabs, forKey: .tabs)
default:
fatalError("unknown RenderBlockContent case in encode method")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ struct DirectiveIndex {
Row.self,
Options.self,
Small.self,
TabNavigator.self,
]

private static let topLevelTutorialDirectives: [AutomaticDirectiveConvertible.Type] = [
Expand Down
103 changes: 103 additions & 0 deletions Sources/SwiftDocC/Semantics/Reference/TabNavigator.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
This source file is part of the Swift.org open source project

Copyright (c) 2022 Apple Inc. and the Swift project authors
Licensed under Apache License v2.0 with Runtime Library Exception

See https://swift.org/LICENSE.txt for license information
See https://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import Foundation
import Markdown

/// A container directive that arranges content into a tab-based layout.
///
/// Create a new tab navigator by writing a `@TabNavigator` directive that contains child
/// `@Tab` directives.
///
/// ```md
/// @TabNavigator {
/// @Tab("Powers") {
/// ![A diagram with the five sloth power types.](sloth-powers)
/// }
///
/// @Tab("Excerise routines") {
/// ![A sloth relaxing and enjoying a good book.](sloth-exercise)
/// }
///
/// @Tab("Hats") {
/// ![A sloth discovering newfound confidence after donning a fedora.](sloth-hats)
/// }
/// }
/// ```
public final class TabNavigator: Semantic, AutomaticDirectiveConvertible, MarkupContaining {
public let originalMarkup: BlockDirective

/// The tabs that make up this tab navigator.
@ChildDirective(requirements: .oneOrMore)
public private(set) var tabs: [Tab]

static var keyPaths: [String : AnyKeyPath] = [
"tabs" : \TabNavigator._tabs,
]

var childMarkup: [Markup] {
return tabs.flatMap(\.childMarkup)
}

@available(*, deprecated,
message: "Do not call directly. Required for 'AutomaticDirectiveConvertible'."
)
init(originalMarkup: BlockDirective) {
self.originalMarkup = originalMarkup
}
}

extension TabNavigator {
/// A container directive that holds general markup content describing an individual
/// tab within a tab-based layout.
public final class Tab: Semantic, AutomaticDirectiveConvertible, MarkupContaining {
public let originalMarkup: BlockDirective

/// The title that should identify the content in this tab when rendered.
@DirectiveArgumentWrapped(name: .unnamed)
public private(set) var title: String

/// The markup content in this tab.
@ChildMarkup(numberOfParagraphs: .oneOrMore, supportsStructure: true)
public private(set) var content: MarkupContainer

static var keyPaths: [String : AnyKeyPath] = [
"title" : \Tab._title,
"content" : \Tab._content,
]

var childMarkup: [Markup] {
return content.elements
}

@available(*, deprecated,
message: "Do not call directly. Required for 'AutomaticDirectiveConvertible'."
)
init(originalMarkup: BlockDirective) {
self.originalMarkup = originalMarkup
}
}
}

extension TabNavigator: RenderableDirectiveConvertible {
func render(with contentCompiler: inout RenderContentCompiler) -> [RenderContent] {
let renderedTabs = tabs.map { tab in
return RenderBlockContent.TabNavigator.Tab(
title: tab.title,
content: tab.content.elements.flatMap { markupElement in
return contentCompiler.visit(markupElement) as! [RenderBlockContent]
}
)
}

let renderedNavigator = RenderBlockContent.TabNavigator(tabs: renderedTabs)
return [RenderBlockContent.tabNavigator(renderedNavigator)]
}
}
40 changes: 40 additions & 0 deletions Sources/SwiftDocC/SwiftDocC.docc/Resources/RenderNode.spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,9 @@
{
"$ref": "#/components/schemas/Row"
},
{
"$ref": "#/components/schemas/TabNavigator"
},
{
"$ref": "#/components/schemas/Aside"
},
Expand Down Expand Up @@ -595,6 +598,43 @@
}
}
},
"TabNavigator": {
"type": "object",
"required": [
"kind",
"tabs"
],
"properties": {
"kind": {
"type": "string",
"enum": ["tabNavigator"]
},
"tabs": {
"type": "array",
"items": {
"$ref": "#/components/schemas/Tab"
}
}
}
},
"Tab": {
"type": "object",
"required": [
"content",
"title"
],
"properties": {
"title": {
"type": "string"
},
"content": {
"type": "array",
"items": {
"$ref": "#/components/schemas/RenderBlockContent"
}
}
}
},
"Aside": {
"type": "object",
"required": [
Expand Down
5 changes: 5 additions & 0 deletions Tests/SwiftDocCTests/Indexing/RenderIndexTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,11 @@ final class RenderIndexTests: XCTestCase {
"title" : "My Article",
"icon" : "plus.svg",
"type" : "article"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the test is now poorly named, since it tests more than CustomIcons at this point. In my mind there are two alternatives. Either we break off the tests into individual tests and instead of XCTAssertEqual on the full string, we assert on the presence of individual elements. alternatively we just rename this test to testRenderIndexGenerationWithBookLikeContent although I am not sure I like to refer to all of these as being book-like since I can see things like this new tab navigator being useful for large amounts of related screenshots in an article within conceptual documentation for an app or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I disagree. I think it's still only testing custom icons- if the navigator index doesn't include just regular article pages in it then something has gone really wrong.

I just had to update the test to reflect the content of the DocC catalog since I added an additional page for a different test in RenderNodeTranslatorTests but the main thing this is testing for is still the special page that customizes an icon.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see what you mean, I think it's fine for this review then. FTR though I think that for someone new to the code base without the context of this PR, it is less clear what the test is checking for. We should come up with a better way of testing individual render index features without having to do this all the time.

{
"path" : "\/documentation\/bestbook\/tabnavigatorarticle",
"title" : "Tab Navigator Article",
"type" : "article"
}
]
},
Expand Down
42 changes: 42 additions & 0 deletions Tests/SwiftDocCTests/Rendering/RenderNodeTranslatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,48 @@ class RenderNodeTranslatorTests: XCTestCase {
)
}

func testTabNavigator() throws {
let (bundle, context) = try testBundleAndContext(named: "BookLikeContent")
let reference = ResolvedTopicReference(
bundleIdentifier: bundle.identifier,
path: "/documentation/BestBook/TabNavigatorArticle",
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 discussion = try XCTUnwrap(
renderNode.primaryContentSections.first(
where: { $0.kind == .content }
) as? ContentRenderSection
)

guard case let .tabNavigator(tabNavigator) = discussion.content.dropFirst().first else {
XCTFail("Expected to find tab as first child.")
return
}


guard tabNavigator.tabs.count == 3 else {
XCTFail("Expected to find a tab navigator with '3' tabs")
return
}

XCTAssertEqual(tabNavigator.tabs[0].title, "Powers")
XCTAssertEqual(tabNavigator.tabs[1].title, "Exercise routines")
XCTAssertEqual(tabNavigator.tabs[2].title, "Hats")

XCTAssertEqual(tabNavigator.tabs[0].content.count, 1)
XCTAssertEqual(tabNavigator.tabs[1].content.count, 2)
XCTAssertEqual(tabNavigator.tabs[2].content.count, 1)
}

func testCustomPageImage() throws {
let (bundle, context) = try testBundleAndContext(named: "BookLikeContent")
let reference = ResolvedTopicReference(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ class DirectiveIndexTests: XCTestCase {
"Small",
"Snippet",
"Stack",
"Tab",
"TabNavigator",
"TechnologyRoot",
"TopicsVisualStyle",
"Tutorial",
Expand All @@ -53,6 +55,7 @@ class DirectiveIndexTests: XCTestCase {
[
"Row",
"Small",
"TabNavigator",
]
)
}
Expand Down
Loading