feat: add SEO microdata for doc breadcrumbs #6697
Conversation
| <span className={className}>{children}</span> | ||
| <span className={className} itemProp="item name"> | ||
| {children} | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| // TODO move to design system folder |
There was a problem hiding this comment.
I'm not even sure what "design system folder" is, and my intuition tells me I don't like this direction😅 Is that like theme-common?
There was a problem hiding this comment.
@Josh-Cena it's a good practice to have a design system for the consistency of any React app
We already have one design system anyway: Infima, but it's a CSS one
As we are in React land, we should create a well-encapsulated abstraction on top (think "infima-react") instead of using infima class names and low-level dom elements everywhere in our theme.
This is related to the theme vision in #6649 (comment)
This is not the same as theme-common, because it's not shared
In general, we should have a good separation of micro layouts (https://web.dev/learn/design/micro-layouts/) vs macro layouts (https://web.dev/learn/design/macro-layouts/) and try to keep technical code out of these
Design system components are particularly suited to be swizzled (eject + copy)
Component users should not have to think about classes, but only props
The design system should be well-encapsulated and prevent impossible/arbitrary UI states
You might also like:
https://www.codercto.com/a/106766.html
https://blog.twitter.com/engineering/en_us/topics/infrastructure/2019/buildingfasterwithcomponents
|
✔️ [V2] 🔨 Explore the source changes: 3ec32b4 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6225c78fd02cc200077c1a11 😎 Browse the preview: https://deploy-preview-6697--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6697--docusaurus-2.netlify.app/ |
|
Size Change: 0 B Total Size: 791 kB ℹ️ View Unchanged
|
| <Link className={className} href={href}> | ||
| {children} | ||
| <Link className={className} href={href} itemProp="item"> | ||
| <span itemProp="name">{children}</span> |
There was a problem hiding this comment.
Do we really need the extra span here and why?
Can't we apply "name" to the link directly?
There was a problem hiding this comment.
Yes, I've experimented. Without the span, the name is not correctly recognized.
| @@ -71,13 +79,17 @@ export default function DocBreadcrumbs(): JSX.Element | null { | |||
| styles.breadcrumbsContainer, | |||
| )} | |||
| aria-label="breadcrumbs"> | |||
| <ul className="breadcrumbs"> | |||
| <ul | |||
There was a problem hiding this comment.
similarly, I didn't do it yet but this must be encapsulated under a <Breadcrumbs> component
packages/docusaurus-theme-classic/src/theme/DocBreadcrumbs/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/DocBreadcrumbs/index.tsx
Outdated
Show resolved
Hide resolved
|
Will refactor it later, absolutely, I just pushed an initial version that seemed to work after countless trials😅 I will still experiment if we can get the "invalid ID" warning fixed in this PR, so we don't ship any bad practices. |
|
Going to merge and see if anyone complains... |
Motivation
Continuation of #6517
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Test on this page: https://search.google.com/test/rich-results
With the following URLs: