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

sidebar: genericize the component and add improvements #277

Merged
merged 3 commits into from
Sep 2, 2024

Conversation

thimy
Copy link
Member

@thimy thimy commented Aug 29, 2024

The sidebar is currently under the Help namespace but we will need it for the upcoming guides so I'm putting it under Shared.

In the mean time I also improve some accessibility and responsive stuff.

@thimy thimy requested a review from fbraure August 29, 2024 12:44
@thimy thimy self-assigned this Aug 29, 2024
Copy link

netlify bot commented Aug 29, 2024

Deploy Preview for bump-content-hub ready!

Name Link
🔨 Latest commit c736506
🔍 Latest deploy log https://app.netlify.com/sites/bump-content-hub/deploys/66d5b2aef074da0008f9349b
😎 Deploy Preview https://deploy-preview-277--bump-content-hub.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@thimy thimy mentioned this pull request Aug 29, 2024
src/_components/shared/sidebar/item.erb Outdated Show resolved Hide resolved
@@ -1,5 +1,5 @@
layout: help
sidebar: true
page_class: help
page_class: help with-sidebar
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry I'm not familiar with this can you explain what is it for?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are css classes that will be applied to the body of all the pages contained in the same folder as _defaults.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation

Copy link
Contributor

@fbraure fbraure left a comment

Choose a reason for hiding this comment

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

I'm left non blocking tiny suggestions/comments.
May you create a separate dedicated commit for the move from Help to Shared ?

@thimy thimy requested a review from fbraure August 30, 2024 07:55
@thimy thimy force-pushed the genericize-sidebar branch from 2ddd191 to c736506 Compare September 2, 2024 12:42
Copy link
Contributor

@fbraure fbraure left a comment

Choose a reason for hiding this comment

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

LGTM.
Thanks for the review, it's clearer and simplifies any reverts with separate commits.

@@ -1,5 +1,5 @@
layout: help
sidebar: true
page_class: help
page_class: help with-sidebar
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation

@thimy thimy merged commit 29fe3fd into bump-sh:main Sep 2, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants