-
Notifications
You must be signed in to change notification settings - Fork 14
Fix sidebar component to render slot 'before' #405
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
Conversation
✅ Deploy Preview for bump-content-hub ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
cb889fa to
d75bc05
Compare
| <% if resource.data.sidebar_title.present? %> | ||
| <% sidebar.slot :before do %> | ||
| <h1> | ||
| <h2> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have another h1 in this page ?
It can be a SEO leak to not have a h1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's because there is already a h1 on this page that I modified to h2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for example here https://deploy-preview-405--bump-content-hub.netlify.app/guides/openapi/specification/v3.1/introduction/what-is-openapi/
h1 is 'What is OpenAPI?'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And before this PR, the 'h1' title with content resource.data.sidebar_title was never rendered (because slot was missing)
fbraure
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
With this preliminary PR, we display the missing title h2 in sidebar component, when provided via slot `before`. - Display slot 'before' if provides, with `<%= slotted :before %>` - Favor tag h2 for this slot, to have only one h1 per page This PR will be necessary to display a OpenAPI version select button, and prepare the release of version v3.2
d75bc05 to
0529c08
Compare
Since #405, slot 'before' is visible, with h2 tag. But margin-top has to be removed to maintain same vertical spacing.
Since #405, slot 'before' is visible, with h2 tag. But margin-top has to be removed to maintain same vertical spacing.
With this preliminary PR,
we display the missing title in sidebar component, when provided via slot
before.<%= slotted :before %>This PR will be necessary to display a OpenAPI version select button, and prepare the release of version v3.2 (cf #404 )
compliant with Figma
before:

now => visible here https://deploy-preview-405--bump-content-hub.netlify.app/guides/openapi/specification/v3.1/introduction/what-is-openapi/
related to https://github.com/bump-sh/bump/issues/7697