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

Split docs into pages, update examples and other details #304

Merged
merged 8 commits into from Mar 28, 2018

Conversation

Projects
None yet
2 participants
@silvenon
Collaborator

silvenon commented Mar 16, 2018

Hi! This is a work in progress, but I'm opening it now to start a discussion. Basically I would like to help a lot with the documentation. My idea would be to first split the documentation into separate pages so it's easier to navigate. Afterwards I would like to improve some examples and recommend using CSSTransition instead of Transition for animating CSS properties because of the reflow hack, I wrote a blog post about it because I noticed quite a few of issues like this one.

Here's a preview of what I got so far.

rtg-docs-split-pages

I also wanted to update the development environment:

  • I renamed .eslintrc files to .eslintrc.yml to get syntax highlighting (they were already written in YAML)
  • I fixed some ESLint errors and added some more based on conventions of the existing code
  • more changes to come to make development and contribution easier

What do you think?

@jquense

This comment has been minimized.

Collaborator

jquense commented Mar 16, 2018

Hey this is awesome! Thanks for taking the time to work on this. I think your plan sounds great

@silvenon

This comment has been minimized.

Collaborator

silvenon commented Mar 17, 2018

Great! Would you be ok if I potentially switched Bootstrap with Semantic UI? The reason I'm asking is primarily because we have to add tons of classes and other attributes which could otherwise be abstracted away, for example look at the code required for one toggle button for mobile navigation:

<button className='navbar-toggler' type='button' data-toggle='collapse' data-target='#navbarSupportedContent' aria-controls='navbarSupportedContent' aria-expanded='false' aria-label='Toggle navigation'>
  <span className='navbar-toggler-icon' />
</button>

Also, we would need to add global jQuery for any JS functionality, like the toggle button above.

@jquense

This comment has been minimized.

Collaborator

jquense commented Mar 17, 2018

I'm not familiar with semantic ui other than I know it's a ui library, I am a maintainer if react-bootstrap tho so I'm a lot more familiar with it.

Are we using RB here it did I just slap on normal bootstrap? RB should equally abstract a lot a way, including many a11y bits

@silvenon

This comment has been minimized.

Collaborator

silvenon commented Mar 17, 2018

We currently have normal Bootstrap. I initially went for your react-bootstrap, but it uses Bootstrap 3, understandably, because v4 isn't stable yet. However, Bootstrap 3 looks… old. 😅😅😅 Would you be ok with reactstrap which implements Bootstrap 4? React Transition Group's documentation is a really simple site, so I don't think we can go wrong here, I just want to make the code as readable as possible while keeping the same look.

@jquense

This comment has been minimized.

Collaborator

jquense commented Mar 19, 2018

I here that, however as it's a docs page I think we should optimize for maintainability and thats best achieved for me by using react-bootstrap and maybe some css tweaks on the style, all things being equal :)

@silvenon

This comment has been minimized.

Collaborator

silvenon commented Mar 19, 2018

You're right! I realized how silly I was, I'll switch to React Bootstrap and send you and update soon.

silvenon added some commits Mar 19, 2018

Add .yml extension to ESLint config files
They are already written in YAML format, so this gives us syntax
highlighting.
Implement Prettier through ESLint
Currently only for documentation files.

@silvenon silvenon force-pushed the silvenon:docs branch from a565a13 to e32c53f Mar 21, 2018

@silvenon silvenon changed the title from [WIP] Splitting docs into pages, updating examples and other stuff to Split docs into pages, update examples and other details Mar 21, 2018

@silvenon

This comment has been minimized.

Collaborator

silvenon commented Mar 21, 2018

Done! It's not perfect, but it should be much clearer and easier to navigate now. Examples now use react-bootstrap and are more complex.

I strongly recommend forking my CodeSandbox examples into your own account and updating <iframe> URLs, to ensure stability.

@jquense

This comment has been minimized.

Collaborator

jquense commented Mar 21, 2018

awesome! thanks so much

* <iframe src="https://codesandbox.io/embed/kw8z6pp9zo?autoresize=1&fontsize=12&hidenavigation=1&moduleview=1" style="width:100%; height:500px; border:0; border-radius: 4px; overflow:hidden;" sandbox="allow-modals allow-forms allow-popups allow-scripts allow-same-origin"></iframe>
* ---
*
* **Note**: `Transition` is platform-agnostic while `CSSTransition`

This comment has been minimized.

@jquense

jquense Mar 21, 2018

Collaborator

this is a really great note, i've wanted to add for a while. It might be off tho in the Transition component, since that's the one folks use incorrectly all the time, so may be likely to see it.

I'd also smooth the recommendation out a bit. I think it is ok and often appropriate to use Transition directly for css animations, but it's fraught if you don't know what you're doing. It might be worth noting that this is a common problem, and if you wnat to use Transtion you need to work around it, or use CSSTransition if it fits your needs.

This comment has been minimized.

@silvenon

silvenon Mar 21, 2018

Collaborator

Good point. I modified it a bit and added it to Transition page instead.

silvenon added some commits Mar 19, 2018

Split components into pages
- use react-bootstrap to add navigation
- auto-generate component pages using Gatsby Node API

@silvenon silvenon force-pushed the silvenon:docs branch from e32c53f to fe2a2a2 Mar 21, 2018

@jquense jquense merged commit 277db71 into reactjs:master Mar 28, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jquense

This comment has been minimized.

Collaborator

jquense commented Mar 28, 2018

thanks a ton!

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