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

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

Merged
merged 8 commits into from Mar 28, 2018
Merged

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

merged 8 commits into from Mar 28, 2018

Conversation

silvenon
Copy link
Collaborator

@silvenon 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
Copy link
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
Copy link
Collaborator Author

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
Copy link
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
Copy link
Collaborator Author

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
Copy link
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
Copy link
Collaborator Author

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

They are already written in YAML format, so this gives us syntax
highlighting.
Currently only for documentation files.
@silvenon silvenon changed the title [WIP] Splitting docs into pages, updating examples and other stuff Split docs into pages, update examples and other details Mar 21, 2018
@silvenon
Copy link
Collaborator Author

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
Copy link
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`
Copy link
Collaborator

@jquense jquense Mar 21, 2018

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

@jquense jquense merged commit 277db71 into reactjs:master Mar 28, 2018
@jquense
Copy link
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants