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: Add table of contents and tabs group to style guide #669

Closed
wants to merge 4 commits into from
Closed

Conversation

srini047
Copy link
Contributor

What kind of change does this PR introduce?

Update Style Guide

Issue Number:

Screenshots/videos:

image

@srini047 srini047 requested a review from a team as a code owner April 21, 2024 06:44
Copy link
Contributor Author

@srini047 srini047 left a comment

Choose a reason for hiding this comment

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

Mentions a blocker and clarification. Any help on blocker appreciated.

styles/globals.css Show resolved Hide resolved
[tabs-start "label"]
[tab "name"]
message to show
```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I add this line:

[tabs-end]

This gets treated as a tab group and gets rendered rather than displaying as a code block. Any help on this appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

Adding one newline [enter] might help in this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DhairyaMajmudar Is this what you meant?
image

If so it is still the same.
image

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, add newline before [tab-end]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still the same😢

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added newline before, after and both. All three yeild the same result as above.

Copy link
Member

Choose a reason for hiding this comment

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

let me check if I can find any another solution 👍🏻

@DhairyaMajmudar
Copy link
Member

@srini047 pls. have a look on this solution now its working correctly.

@srini047
Copy link
Contributor Author

@srini047 pls. have a look on this solution now its working correctly.

@DhairyaMajmudar Yes this works fine to render but there's a catch. Say I copy paste this and try to render the component, this doesn't work. Explicitly I need to remove the spaces, only then the implementation works.
P.S.: Direct copy paste doesnt work.

If this is fine then I'm ready to update. What's ur view on this.

@gregsdennis
Copy link
Member

I'm not fond of how the example ToC renders. Is there anything we can do to make it look more like a ToC and not just s bunch of "example" links?

@srini047
Copy link
Contributor Author

I'm not fond of how the example ToC renders. Is there anything we can do to make it look more like a ToC and not just s bunch of "example" links?

@gregsdennis Currently I have set the depth as 5 and it renders the contents for the specific file. So it shows like that. Let me try other possible methods and let you know if any that is better.

Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

Hi @srini047. I left some comments regarding changes of issue #571 unnecessarily added here. Please remove the changes in the css and try to keep each feature in a different branch to make sure the PRs contain the appropriate content.

/* margin-right: 10px; */
/* display: flex; */
/* border: 1px solid #E3E8EE;
/* background-color: transparent !important; */
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no need to modify the css to just add to new tags to the style guide. I think your are adding here your work to modify the design of the table of contents but we should avoid it. Please use two different branches to fix each issue.

@benjagm benjagm added the Status: In Progress This issue is being worked on, and has someone assigned. label Apr 23, 2024
@@ -1,145 +1,230 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

Why was all of this content replaced with what looks like primarily CSS? Where is this content now?

@srini047
Copy link
Contributor Author

@gregsdennis @benjagm Will make a new PR. Closing this due to incorrect commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress This issue is being worked on, and has someone assigned.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[📝 Docs]: Add Table of Contents and Tabs Group to the Markdown Style Guide
4 participants