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: code block tab #1063

Merged
merged 8 commits into from
Jan 24, 2019
Merged

Conversation

fiennyangeln
Copy link
Contributor

Motivation

#103

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

  1. I currently changed custom-pages in the 1.0.15 version so that I can visualize how it looks like, I plan to remove it later. So, go to /docs/en/1.0.15/custom-pages:

screen shot 2018-10-24 at 11 12 02 pm

2. Try to click on the tab 3. See if the other code tab changes too or not 4. Try to resize the width of the browser to be abit smaller to see how it looks like if it overflow

Related PRs

Null

Todo

  1. Is the design suitable? Should we handle older browser that might not support horizontal scroll?
  2. Should we have other design for smaller device?
  3. Is the way I add the onclick in the js below the copy field looks right?

where it ends
- I try using DOCUSAURUS_CODE_TABS to mark the start
- Use TAB_TITLE to mark the title of the tab
- END_TAB to mark the end of that tab
- END_DOCUSAURUS_CODE_TABS to mark the end of the whole code blocks
then parse using regex and render accordingly
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 24, 2018
Copy link
Contributor

@JoelMarcey JoelMarcey left a comment

Choose a reason for hiding this comment

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

I think this is a fine way to approach the problem. I am definitely ok with having markdown markers that represent the various code tabs like you have. I will comment inline.

@@ -59,6 +98,8 @@ class Doc extends React.Component {
);
}

// const a = this.props.content.split(/(DOCUSAURUS_CODE_TABS)(.*?)(END_DOCUSAURUS_CODE_TABS)/gms)
// console.log(a.length, a);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove these.

@@ -34,12 +34,108 @@ root-of-repo
│ └── static
```


Copy link
Contributor

Choose a reason for hiding this comment

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


## Codeblocks in multiple languages

DOCUSAURUS_CODE_TABS
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a clever way to do this.

@@ -34,12 +34,108 @@ root-of-repo
│ └── static
```


## Codeblocks in multiple languages

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add some wording about what is going on here.

Of course, you are also free to write your own pages. It is strongly suggested that you at least have an index page, but none of the pages provided are mandatory to include in your site. More information on how to use the provided components or include your own can be found [here](api-pages.md). Information on how to link to your different pages in the header navigation bar can be found [here](guides-navigation.md).

> If you want your page to show up in your navigation header, you will need to update `siteConfig.js` to add to the `headerLinks` element. e.g., `{ page: 'about-slash', label: 'About/' }`,

## Adding Static Pages

DOCUSAURUS_CODE_TABS
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure you need this all this example here if you add the documentation to https://github.com/facebook/Docusaurus/blob/master/docs/api-doc-markdown.md

@@ -44,4 +44,25 @@ window.addEventListener('load', function() {
textEl.textContent = 'Copy';
}, 2000);
});

// add event listener for all tab
document.querySelectorAll('.nav-link').forEach(function(el) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We will need to make sure that this is installed as part of the Docusaurus install process -- e.g., docusaurus-init

@JoelMarcey
Copy link
Contributor

@endiliey @yangshun - is the v1 Netlify preview broken?

@endiliey
Copy link
Contributor

I think the test are failing also. Might not be because of CI

}

.nav-tabs::-webkit-scrollbar {
display: none; // Safari and Chrome
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 guess i should not put comment on this :/

Copy link
Contributor

Choose a reason for hiding this comment

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

In CSS comments should be /* */ not //

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Oct 25, 2018

Deploy preview for docusaurus-preview ready!

Built with commit 9cb10e2

https://deploy-preview-1063--docusaurus-preview.netlify.com

@endiliey
Copy link
Contributor

document
.querySelectorAll(`.tab-pane[data-group=${groupId}]`)
.forEach(function(el) {
el.classList.remove('active');
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldnt be placed here because not every docusaurus user has this file

Copy link
Contributor

Choose a reason for hiding this comment

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

@endiliey I suggested that we add it as part of docusaurus-init. But you might be right, since that is more of a custom file. Could put it in v1/lib/static maybe?

Copy link
Contributor Author

@fiennyangeln fiennyangeln Oct 26, 2018

Choose a reason for hiding this comment

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

Hmm yeah I'm confused where to put it actually 😯 I put it there to mimic the code blocks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I make something like this? https://gist.github.com/yangshun/55db997ed0f8f4e6527571fc3bee4675 and put all the related css outside of the lib?

Copy link
Contributor

Choose a reason for hiding this comment

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

put it in v1/lib/static and modify the Head.js to always include it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@endiliey @JoelMarcey made the changes:D please review!

Copy link
Contributor

@JoelMarcey JoelMarcey left a comment

Choose a reason for hiding this comment

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

Let's ship this.

@endiliey, @yangshun I will let you make the final approval and merge.

@endiliey endiliey changed the title WIP: Code block tab feat: Code block tab Nov 7, 2018
@endiliey
Copy link
Contributor

endiliey commented Nov 7, 2018

I'm currently not well so my review will come late this time.

Copy link
Contributor

@endiliey endiliey left a comment

Choose a reason for hiding this comment

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

Seems like a great feature for me

https://deploy-preview-1063--docusaurus-preview.netlify.com/docs/en/next/doc-markdown#code-tabs
image

Kinda dislike the syntax to do the tabbing though :(. Is it possible to make it simpler ?. I'm wondering what are the popular syntax are like out there.

I am wondering if END_TAB is really necessary ? Can we do better ?

Current
image

Maybe ? (at least without END_TAB it looks nicer
image

@fiennyangeln
Copy link
Contributor Author

I will give the above a try! 💃

@fiennyangeln fiennyangeln force-pushed the feature/code-blocks-tab branch 2 times, most recently from bc5fc37 to f17cff3 Compare November 29, 2018 10:58
@endiliey
Copy link
Contributor

endiliey commented Dec 7, 2018

Is this PR ready or still WIP ? LGTM on a glance.

Will review after that

@JoelMarcey
Copy link
Contributor

This is ready to go, I believe.

@fiennyangeln
Copy link
Contributor Author

@endiliey feel free to review.

@t00f
Copy link

t00f commented Jan 4, 2019

We tested it and are waiting for the official release 👍

@CyrilPeponnet
Copy link

Works like a charm, would be awesome to be part of next release.

@yangshun yangshun changed the title feat: Code block tab feat: code block tab Jan 23, 2019
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Nice work @fiennyangeln!

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

Successfully merging this pull request may close these issues.

8 participants