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

feat: toc start expanded #197

Merged
merged 2 commits into from
Nov 17, 2020
Merged

feat: toc start expanded #197

merged 2 commits into from
Nov 17, 2020

Conversation

mallachari
Copy link
Contributor

Needed by stoplightio/elements#722

Adds support for initially expanded ToC items.

@mallachari mallachari requested review from a team and marcelltoth and removed request for a team November 17, 2020 11:52
Comment on lines 132 to 145
// expand items marked as initially expanded and its children
React.useEffect(() => {
const itemsToExpand = contents.filter(item => item.startExpanded);
const childrenToExpand = flatMap(itemsToExpand, item =>
findDescendantIndices(item.depth ?? 0, contents.indexOf(item), contents.slice(contents.indexOf(item) + 1)),
);

setExpanded(current => ({
...current,
...Object.fromEntries(itemsToExpand.map(index => [index, true])),
...Object.fromEntries(childrenToExpand.map(index => [index, true])),
}));
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't buy why a prop called startExpanded would make children expanded.

Also is that required to adhere to the spec? It only wants you to expand individual APIs, not groups within, so that should be enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

And instead of hacking a useEffect, you could move the logic to line 108:

  const [expanded, setExpanded] = React.useState({});

and replace the empty initializer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understood Apis should start expanded as fully expanding its content with groups, but it might be just first level depth.
If we don't need to expand children then it will be simpler and without useEffect

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need it all the way, that can get very messy quick

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to be less greedy when expanding

Copy link
Contributor

@marcelltoth marcelltoth left a comment

Choose a reason for hiding this comment

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

👋

@mallachari mallachari merged commit 8134d01 into beta Nov 17, 2020
@mallachari mallachari deleted the feat/toc-start-expanded branch November 17, 2020 13:02
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version 3.0.0-beta.39 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants