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 expand and allow custom root section options #1689

Merged
merged 6 commits into from
Sep 9, 2020
Merged

Add expand and allow custom root section options #1689

merged 6 commits into from
Sep 9, 2020

Conversation

josephrexme
Copy link
Contributor

This addresses #1674 which allows user set and expand setting for each root section. It also sends all custom options from the user through to be usable

Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request! Please update the docs and tests.

@josephrexme
Copy link
Contributor Author

Thanks for taking a look @sapegin . I have just added tests. The syntax highlighting test here that I modified was failing even though I hadn't touched it so I made it look like the value it was expecting. I can revert that if it's not needed. I don't know if I also had to include an updated snapshot

@codecov
Copy link

codecov bot commented Sep 8, 2020

Codecov Report

Merging #1689 into master will not change coverage.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
src/loaders/utils/getSections.ts 100.00% <ø> (ø)
...rsg-components/TableOfContents/TableOfContents.tsx 100.00% <100.00%> (ø)

@sapegin
Copy link
Member

sapegin commented Sep 8, 2020

The syntax highlighting test here that I modified was failing even though I hadn't touched it so I made it look like the value it was expecting. I can revert that if it's not needed. I don't know if I also had to include an updated snapshot

This is because Prism was updated at some point, and the snapshot is already fixed in master.

@@ -136,6 +136,12 @@ Defines the initial state of the example code tab:
- `hide`: hide the tab and it can´t be toggled in the UI.
- `expand`: expand the tab by default.

## `expand`
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused: this isn't a global configuration option, right? If so, we should document it here:

https://github.com/styleguidist/react-styleguidist/blob/master/docs/Components.md#sections

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

@@ -88,6 +88,7 @@ export function processSection(
}

return {
...section,
name: section.name,
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this line anymore, and also description and external below.

@josephrexme
Copy link
Contributor Author

The syntax highlighting test here that I modified was failing even though I hadn't touched it so I made it look like the value it was expecting. I can revert that if it's not needed. I don't know if I also had to include an updated snapshot

This is because Prism was updated at some point, and the snapshot is already fixed in master.

To be clear, should I undo my change with the highlightCode.spec.ts file?

@sapegin
Copy link
Member

sapegin commented Sep 8, 2020

Try to merge master into your branch.

@sapegin sapegin merged commit 3ab6f8d into styleguidist:master Sep 9, 2020
@styleguidist-bot
Copy link
Collaborator

🎉 This PR is included in version 11.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@josephrexme
Copy link
Contributor Author

Thanks for the reviews and accepting my PR @sapegin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants