-
-
Notifications
You must be signed in to change notification settings - Fork 744
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
Adds MDX support and a Tab component for the different examples #2810
Conversation
Awesome, can't wait for this one :) |
Cannot wait to see it in action 👍 |
917af8f
to
e84e988
Compare
this is really cool!! |
we could even play around with a dropdown in the mobile version. this may scale better with more than three tabs |
That is certainly an improvement. I decided not to do it yet, as we currently don't have more than three tabs and I would improve it when the scenario comes up :). |
Very cool to see progress on the tabs component, just let me know when I could start the review. |
7e0ec01
to
69aab20
Compare
69aab20
to
e09cdcf
Compare
@rstaib ready for review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still some design issues, all described in this follow up:
#2930
Everything else is fine!
Added a new commit 7b770ce which fixed the horizontal scrolling in the pre tag for small viewports (i.e. language indicator and copy button no longer scroll) and added some margin when a tab begins with a paragraph. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/AzurePipelines Run |
Azure Pipelines successfully started running 2 pipeline(s). |
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! Just a few comments.
@fredericbirke at the moment we should not have any text in tabs. If so, then we should probably rewrite the section. |
@fredericbirke can you point me to the file where you found text inside a tab!? thanks |
@rstaib |
@fredericbirke we should rewrite that. But for now we keep it look ugly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good! Just one more thing :-D Could you please run yarn format
so that prettier takes care of code styling.
remark
-Plugins.Tabs
component and a tailoredExampleTabs
component for the domain with the fixed three different approaches.TODO after deployment (prod/staging):
@darth-knoppix/gatsby-plugin-feed
still works with thehtml
field, given that we now process mdx and feed reader look as clean as before.Couldn't fix, waiting for new version:
Invalid DOM property 'xmlnsXLink'. Did you mean 'xmlnsXlink'
from the generated mermaid graphic. This is supposedly fixed in mdx 2.0 MDX needs to map some camelcase properties to what React expects mdx-js/mdx#1148