-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
docs(ComponentSidebar): add sidebar navigation #2070
Conversation
4e5dc6c
to
f51296b
Compare
Codecov Report
@@ Coverage Diff @@
## master #2070 +/- ##
=======================================
Coverage 99.73% 99.73%
=======================================
Files 152 152
Lines 2656 2656
=======================================
Hits 2649 2649
Misses 7 7 Continue to review full report at Codecov.
|
4adc6fa
to
6de75d0
Compare
This is great, been on my list for a long :) I think this can be done by using the already existent Note, in the future, I've been considering the use of dynamic imports for each component route. This way, we only load the docs people need. It will make the docs much lighter and faster to load. Along with this change, I'd like to split up |
6de75d0
to
c9740a2
Compare
…React into docs/sidebar # Conflicts: # .gitignore # docs/app/Components/ComponentDoc/ComponentProp/ComponentPropEnum.js # gulp/plugins/util/index.js
@@ -70,6 +75,10 @@ class ComponentExample extends Component { | |||
}) | |||
} | |||
|
|||
shouldComponentUpdate(nextProps, nextState) { |
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.
Small optimization. Without it, all examples will be rerendered
NamingMoved to #2123.
|
dd1497b
to
0dafcf9
Compare
I will try to take a look and comment. I've marked this PR for review to remind me. |
841249b
to
0edc433
Compare
0edc433
to
da68908
Compare
@levithomason Don't pay attention to the refactoring, I moved it into #2123. However, question about |
…React into docs/sidebar # Conflicts: # docs/app/Components/ComponentDoc/ComponentDoc.js # docs/app/Components/ComponentDoc/ComponentDocHeader.js # docs/app/Components/ComponentDoc/ComponentDocLinks.js # docs/app/Components/ComponentDoc/ComponentDocSee.js # docs/app/Components/ComponentDoc/ComponentProps/ComponentProps.js # docs/app/Components/ComponentDoc/ComponentProps/ComponentPropsComponents.js # docs/app/Components/ComponentDoc/ComponentProps/ComponentPropsDescription.js # docs/app/Components/ComponentDoc/ComponentTable/ComponentTable.js # docs/app/utils/index.js # yarn.lock
3376baf
to
e1f6628
Compare
e1f6628
to
5a483cf
Compare
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.
@levithomason I merged this PR with and made the cleanup there. it's ready for review ✌️
However, I can't fix the problem with scrollToAnchor()
. It's easy to reproduce, just pull this branch and try to click on different sidebar items on the Header
page. The scroll sometime will not reach the matching position and the wrong sidebar item will be active. There is a rounding problem, but I don't have any idea how to fix it 🤔
eea1e98
to
1b1f5b3
Compare
I've pushed some usability fixes and would like to do more. The issue you stated still persists. It seems to me the cause is that Visibility expects the element to be passed, while I will come back to this again. I am out of time for today. 👋 |
…React into docs/sidebar # Conflicts: # docs/app/Components/ComponentDoc/ComponentDoc.js
Solved merge conflict there 👍 |
I'm going to merge this and we can iterate on it. Maybe someone will feel motivated and submit a PR as well. |
Released in |
This PR is part of work (#1981, #2012) that improves docs usability and performance.
sidebar
This PR will also add the navigation sidebar like on SUI website.
TODO
scrollToAnchor()