Support custom page icons and image overrides #417

dobromir-hristov · 2022-08-22T16:04:39Z

Bug/issue #, if applicable: 97715925

Summary

Allow overriding the icons and images related to a page. These can be navigator icon, DocumentationHero icon or the Topics links.

Note 1
This PR sort of builds upon the #402, and more particularly the icon overriding. There should be minimal changed needed for both to co-exist.

Dependencies

#402
DocC PR - swiftlang/swift-docc#367

Testing

Use the DocC PR to build an archive with image overrides or use the attached one below

archive_with_icon_override.zip

Steps:

Point docc-render's serve to the archive of your choice
Go to a page that has page overrides, in my case http://localhost:8081/documentation/docc/formatting-your-documentation-content
Assert the icon gets changed in both Navigator and DocumentationHero.
Assert that when visiting http://localhost:8081/documentation/docc/ the page above also has a changed icon in the Topic links.

Checklist

Added tests
Ran npm test, and it succeeded
Updated documentation if necessary

… `/components`

ethan-kusters · 2022-09-01T15:23:45Z

RenderJSON spec for this change is up here: https://github.com/apple/swift-docc/pull/367/files#diff-5a65fbdff488799d5ec3462fe3145b2fa8d19a8e029cd568d12dc9fd5d613b46.

I've added an svgID property that just pulls the first ID we find in the given SVG file to support what we need for the theming infrastructure.

dobromir-hristov · 2022-09-06T09:13:42Z

I've added an svgID property that just pulls the first ID we find in the given SVG file to support what we need for the theming infrastructure.

Awesome. Added support for it.

mportiz08 · 2022-09-09T22:33:04Z

I haven't dug into the code review too much yet, but I started playing with this in combination with the corresponding DocC PR and I'm noticing some potential issues. It seems like only the icon in the navigator is being overwritten, and I think there are a couple of other places where this icon is being shown like the topics section and documentation intro.

Examples:

For those screenshots, I added a new square.svg asset to the bundle and included the following directive in an article like so:

# Formatting Your Documentation Content

@Metadata {
  @PageImage(purpose: icon, source: "square.svg", alt: "A square icon")
}

Enhance your content's presentation with special formatting and styling for text, links, and other page elements.

mportiz08 · 2022-09-09T22:35:34Z

And my new icon looks like this for reference:

<svg viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg" id="square">
  <rect x="2" y="2" width="10" height="10" stroke="currentColor" fill="none"></rect>
</svg>

Let me know if I misunderstood anything about this is expected to work. Thanks!

dobromir-hristov · 2022-09-12T11:34:07Z

Thanks for the review @mportiz08 I used Ethan's DocC branch to do a more real test, with actual data and I fixed the issues you pointed out.

I am not super happy about how I got similar overriding logic in the two icon components, but I was not sure how to extract that away. I dont want to use mixins, yet we need a way to optionally render the override instead of the actual icon, while still applying classes and styling to both.

mportiz08 · 2022-09-12T22:31:25Z

Cool. It looks like the icon is being used in all the right places now that I highlighted in those screenshots.

mportiz08 · 2022-09-12T22:46:59Z

What is the expectation for this feature when combined with the (global) icon overriding from the theme-settings feature?

For example, what if I add an overriding icon for a specific article and also override the default icon for all articles using theme-settings.json? Personally, I would expect the icon specified in the markdown directive to still have priority and only be shown for that one particular article, but I just want to make sure that's how it's expected to work. I could probably test this out, but I'd need to do a test branch to combine this and the theme-settings PR I think since this isn't directly dependent on that branch as of now.

mportiz08 · 2022-09-12T23:05:41Z

I am not super happy about how I got similar overriding logic in the two icon components, but I was not sure how to extract that away. I dont want to use mixins, yet we need a way to optionally render the override instead of the actual icon, while still applying classes and styling to both.

Would a mixin solve the problem and you just prefer not to use them or is there an issue making them tricky to use here? Is there a solution where introducing a new component would simplify things or is there logic that could be moved to the base SVGIcon component to eliminate the repetition? (I'm not sure of the right answer myself off the top of my head, just throwing out some ideas in case something helps)

dobromir-hristov · 2022-09-13T06:25:11Z

What is the expectation for this feature when combined with the (global) icon overriding from the theme-settings feature?

I think as you, that markdown specified icon should have weight over the globally replaced icon.

Would a mixin solve the problem and you just prefer not to use them

A mixin would probably help here, I am just not the biggest fan of them. I will keep you updated.

dobromir-hristov · 2022-09-13T10:33:34Z

OK refactored the duplicated logic out into a provider component, no magic mixins 🤨

Let me know if this is OK.

marinaaisa · 2022-09-13T16:39:47Z

Code looks good. I've been trying to test it using the SlothCreator_with_image_overrides.doccarchive you attached but it seems that the data is not updated or overwritting.
Is the testing steps updated?

dobromir-hristov · 2022-09-14T09:20:04Z

Is the testing steps updated?

I just went over them and re-build an archive of docc docs for you. You can also try out using the docc PR.

marinaaisa

Tested and works great!

dobromir-hristov · 2022-09-15T15:36:31Z

@swift-ci test

@metadata

Adds a new `@PageImage` directive that can be placed inside a page’s @metadata directive block to allow for customizing the images that will be used to represent the page in the navigator, hero, and elsewhere in the UI. For article-only catalogs it’s likely helpful to distinguish between articles with custom icons- in the same way Swift-DocC has different icons for different kinds of symbol pages. `@PageImage` accepts the following parameters: - purpose: The purpose of the image. One of the following: - icon: This image should be used in place of wherever a generic icon for the page would usually be rendered. - card: This image should be used whenever rendering a card representing this page. - source: A string containing the name of an image in your DocC catalog. - alt: (optional) A string containing a description of the image. Example: # Feeding a Sloth @metadata { @PageImage(purpose: icon, source: "leaf.svg", alt: "An icon representing a leaf.") } `@PageImage` is described on the Swift forums here: https://forums.swift.org/t/supporting-more-types-of-documentation-with-swift-docc/59725#pageimage-22 Dependencies: - swiftlang/swift-docc-render#417 Resolves rdar://97739580

@metadata

Adds a new `@PageImage` directive that can be placed inside a page’s @metadata directive block to allow for customizing the images that will be used to represent the page in the navigator, hero, and elsewhere in the UI. For article-only catalogs it’s likely helpful to distinguish between articles with custom icons- in the same way Swift-DocC has different icons for different kinds of symbol pages. `@PageImage` accepts the following parameters: - purpose: The purpose of the image. One of the following: - icon: This image should be used in place of wherever a generic icon for the page would usually be rendered. - card: This image should be used whenever rendering a card representing this page. - source: A string containing the name of an image in your DocC catalog. - alt: (optional) A string containing a description of the image. Example: # Feeding a Sloth @metadata { @PageImage(purpose: icon, source: "leaf.svg", alt: "An icon representing a leaf.") } `@PageImage` is described on the Swift forums here: https://forums.swift.org/t/supporting-more-types-of-documentation-with-swift-docc/59725#pageimage-22 Dependencies: - swiftlang/swift-docc-render#417 Resolves rdar://97739580

@metadata

Adds a new `@PageImage` directive that can be placed inside a page’s @metadata directive block to allow for customizing the images that will be used to represent the page in the navigator, hero, and elsewhere in the UI. For article-only catalogs it’s likely helpful to distinguish between articles with custom icons- in the same way Swift-DocC has different icons for different kinds of symbol pages. `@PageImage` accepts the following parameters: - purpose: The purpose of the image. One of the following: - icon: This image should be used in place of wherever a generic icon for the page would usually be rendered. - card: This image should be used whenever rendering a card representing this page. - source: A string containing the name of an image in your DocC catalog. - alt: (optional) A string containing a description of the image. Example: # Feeding a Sloth @metadata { @PageImage(purpose: icon, source: "leaf.svg", alt: "An icon representing a leaf.") } `@PageImage` is described on the Swift forums here: https://forums.swift.org/t/supporting-more-types-of-documentation-with-swift-docc/59725#pageimage-22 Dependencies: - swiftlang/swift-docc-render#417 Resolves rdar://97739580

@metadata

Adds a new `@PageImage` directive that can be placed inside a page’s `@Metadata` directive block to allow for customizing the images that will be used to represent the page in the navigator, hero, and elsewhere in the UI. For article-only catalogs it’s likely helpful to distinguish between articles with custom icons- in the same way Swift-DocC has different icons for different kinds of symbol pages. `@PageImage` accepts the following parameters: - purpose: The purpose of the image. One of the following: - icon: This image should be used in place of wherever a generic icon for the page would usually be rendered. - card: This image should be used whenever rendering a card representing this page. - source: A string containing the name of an image in your DocC catalog. - alt: (optional) A string containing a description of the image. Example: # Feeding a Sloth @metadata { @PageImage(purpose: icon, source: "leaf.svg", alt: "An icon representing a leaf.") } `@PageImage` is described on the Swift forums here: https://forums.swift.org/t/supporting-more-types-of-documentation-with-swift-docc/59725#pageimage-22 Dependencies: - swiftlang/swift-docc-render#417 Resolves rdar://97739580

@metadata

Adds a new `@PageImage` directive that can be placed inside a page’s `@Metadata` directive block to allow for customizing the images that will be used to represent the page in the navigator, hero, and elsewhere in the UI. For article-only catalogs it’s likely helpful to distinguish between articles with custom icons- in the same way Swift-DocC has different icons for different kinds of symbol pages. `@PageImage` accepts the following parameters: - purpose: The purpose of the image. One of the following: - icon: This image should be used in place of wherever a generic icon for the page would usually be rendered. - card: This image should be used whenever rendering a card representing this page. - source: A string containing the name of an image in your DocC catalog. - alt: (optional) A string containing a description of the image. Example: # Feeding a Sloth @metadata { @PageImage(purpose: icon, source: "leaf.svg", alt: "An icon representing a leaf.") } `@PageImage` is described on the Swift forums here: https://forums.swift.org/t/supporting-more-types-of-documentation-with-swift-docc/59725#pageimage-22 Dependencies: - swiftlang/swift-docc-render#417 Resolves rdar://97739580

@metadata

Adds a new `@PageImage` directive that can be placed inside a page’s `@Metadata` directive block to allow for customizing the images that will be used to represent the page in the navigator, hero, and elsewhere in the UI. For article-only catalogs it’s likely helpful to distinguish between articles with custom icons- in the same way Swift-DocC has different icons for different kinds of symbol pages. `@PageImage` accepts the following parameters: - purpose: The purpose of the image. One of the following: - icon: This image should be used in place of wherever a generic icon for the page would usually be rendered. - card: This image should be used whenever rendering a card representing this page. - source: A string containing the name of an image in your DocC catalog. - alt: (optional) A string containing a description of the image. Example: # Feeding a Sloth @metadata { @PageImage(purpose: icon, source: "leaf.svg", alt: "An icon representing a leaf.") } `@PageImage` is described on the Swift forums here: https://forums.swift.org/t/supporting-more-types-of-documentation-with-swift-docc/59725#pageimage-22 Dependencies: - swiftlang/swift-docc-render#417 Resolves rdar://97739580

@metadata

Adds a new `@PageImage` directive that can be placed inside a page’s `@Metadata` directive block to allow for customizing the images that will be used to represent the page in the navigator, hero, and elsewhere in the UI. For article-only catalogs it’s likely helpful to distinguish between articles with custom icons- in the same way Swift-DocC has different icons for different kinds of symbol pages. `@PageImage` accepts the following parameters: - purpose: The purpose of the image. One of the following: - icon: This image should be used in place of wherever a generic icon for the page would usually be rendered. - card: This image should be used whenever rendering a card representing this page. - source: A string containing the name of an image in your DocC catalog. - alt: (optional) A string containing a description of the image. Example: # Feeding a Sloth @metadata { @PageImage(purpose: icon, source: "leaf.svg", alt: "An icon representing a leaf.") } `@PageImage` is described on the Swift forums here: https://forums.swift.org/t/supporting-more-types-of-documentation-with-swift-docc/59725#pageimage-22 Dependencies: - swiftlang/swift-docc-render#417 Resolves rdar://97739580

@metadata

Adds a new `@PageImage` directive that can be placed inside a page’s `@Metadata` directive block to allow for customizing the images that will be used to represent the page in the navigator, hero, and elsewhere in the UI. For article-only catalogs it’s likely helpful to distinguish between articles with custom icons- in the same way Swift-DocC has different icons for different kinds of symbol pages. `@PageImage` accepts the following parameters: - purpose: The purpose of the image. One of the following: - icon: This image should be used in place of wherever a generic icon for the page would usually be rendered. - card: This image should be used whenever rendering a card representing this page. - source: A string containing the name of an image in your DocC catalog. - alt: (optional) A string containing a description of the image. Example: # Feeding a Sloth @metadata { @PageImage(purpose: icon, source: "leaf.svg", alt: "An icon representing a leaf.") } `@PageImage` is described on the Swift forums here: https://forums.swift.org/t/supporting-more-types-of-documentation-with-swift-docc/59725#pageimage-22 Dependencies: - swiftlang/swift-docc-render#417 Resolves rdar://97739580

Dobromir Hristov added 3 commits August 11, 2022 11:35

refactor: rename NavigatorLeafIcon to TopicTypeIcon. Move location to…

6cd9f6a

… `/components`

feat: support icon overriding

93ed3a2

chore: add tests

ce1f219

dobromir-hristov requested review from mportiz08, ethan-kusters, marinaaisa and hqhhuang August 22, 2022 16:04

dobromir-hristov mentioned this pull request Aug 23, 2022

Add support for @TopicsVisualStyle #419

Merged

3 tasks

ethan-kusters mentioned this pull request Aug 26, 2022

Add page image directive swiftlang/swift-docc#367

Merged

3 tasks

refactor: use provided in RenderJSOn svgID, for image overrides.

9bbcf96

refactor: use images metadata property to override the page image

45297f7

Dobromir Hristov added 2 commits September 12, 2022 12:37

fix: use identifier instead of reference

048e70a

refactor: allow overriding the icon for TopicsLinkBlock

0b9f5b3

Dobromir Hristov added 2 commits September 13, 2022 13:32

refactor: extract away repeated logic

aebfb1f

Merge branch 'main' into dhristov/r97715925-support-custom-page-icon

222ff7f

marinaaisa approved these changes Sep 14, 2022

View reviewed changes

Dobromir Hristov added 2 commits September 15, 2022 18:03

Merge branch 'main' into dhristov/r97715925-support-custom-page-icon

a20324f

Merge branch 'main' into dhristov/r97715925-support-custom-page-icon

ba12c68

dobromir-hristov merged commit 956a495 into swiftlang:main Sep 15, 2022

dobromir-hristov deleted the dhristov/r97715925-support-custom-page-icon branch September 15, 2022 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support custom page icons and image overrides #417

Support custom page icons and image overrides #417

Uh oh!

dobromir-hristov commented Aug 22, 2022 •

edited

Loading

Uh oh!

ethan-kusters commented Sep 1, 2022

Uh oh!

dobromir-hristov commented Sep 6, 2022 •

edited

Loading

Uh oh!

mportiz08 commented Sep 9, 2022

Uh oh!

mportiz08 commented Sep 9, 2022

Uh oh!

dobromir-hristov commented Sep 12, 2022

Uh oh!

mportiz08 commented Sep 12, 2022

Uh oh!

mportiz08 commented Sep 12, 2022

Uh oh!

mportiz08 commented Sep 12, 2022

Uh oh!

dobromir-hristov commented Sep 13, 2022

Uh oh!

dobromir-hristov commented Sep 13, 2022

Uh oh!

marinaaisa commented Sep 13, 2022

Uh oh!

dobromir-hristov commented Sep 14, 2022

Uh oh!

marinaaisa left a comment

Uh oh!

dobromir-hristov commented Sep 15, 2022

Uh oh!

Uh oh!

Support custom page icons and image overrides #417

Support custom page icons and image overrides #417

Uh oh!

Conversation

dobromir-hristov commented Aug 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Dependencies

Testing

Checklist

Uh oh!

ethan-kusters commented Sep 1, 2022

Uh oh!

dobromir-hristov commented Sep 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mportiz08 commented Sep 9, 2022

Uh oh!

mportiz08 commented Sep 9, 2022

Uh oh!

dobromir-hristov commented Sep 12, 2022

Uh oh!

mportiz08 commented Sep 12, 2022

Uh oh!

mportiz08 commented Sep 12, 2022

Uh oh!

mportiz08 commented Sep 12, 2022

Uh oh!

dobromir-hristov commented Sep 13, 2022

Uh oh!

dobromir-hristov commented Sep 13, 2022

Uh oh!

marinaaisa commented Sep 13, 2022

Uh oh!

dobromir-hristov commented Sep 14, 2022

Uh oh!

marinaaisa left a comment

Choose a reason for hiding this comment

Uh oh!

dobromir-hristov commented Sep 15, 2022

Uh oh!

Uh oh!

dobromir-hristov commented Aug 22, 2022 •

edited

Loading

dobromir-hristov commented Sep 6, 2022 •

edited

Loading