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

Documentation: Colors: Add withColors readme with a sample. #7906

Closed

Conversation

jorgefilipecosta
Copy link
Member

Description

This PR adds more extensive documentation for the withColors HOC and provides a sample block making use of it.

@jorgefilipecosta jorgefilipecosta added [Type] Enhancement A suggestion for improvement. [Type] Developer Documentation Documentation for developers labels Jul 11, 2018
@jorgefilipecosta jorgefilipecosta force-pushed the add/withColors-documentation-and-sample branch from 6589ece to c6ad0f6 Compare July 12, 2018 08:49
@chrisvanpatten
Copy link
Contributor

Hey @jorgefilipecosta would you mind if I took a stab at a revision on these docs? I can't push to your PR but I'll branch from it and push a new branch on my fork.

@jorgefilipecosta
Copy link
Member Author

jorgefilipecosta commented Jul 23, 2018

Sure @chrisvanpatten feel free to branch from it 👍 Thank you!

@gziolo gziolo requested a review from a team August 1, 2018 09:18
@tofumatt
Copy link
Member

tofumatt commented Aug 7, 2018

@chrisvanpatten Have you had the chance to push your own branch based on these docs? If yes and I missed it: sorry! I just don't see anything linked from this PR and saw this still open in the review queue.

@gziolo
Copy link
Member

gziolo commented Nov 22, 2018

@jorgefilipecosta what's blocking this one?

@jorgefilipecosta
Copy link
Member Author

I'm not sure if @chrisvanpatten or other person involved in the documentation efforts continued the work in another documentation branch. @chrisvanpatten could you confirm if that's case? If not I will update this PR so we can have new look.

@gziolo
Copy link
Member

gziolo commented Jan 29, 2019

@chrisvanpatten - it looks like it awaits your feedback :)

@jorgefilipecosta - the docs file will have to be moved to a new location /packages/editor/src/components/colors. I don't see any README.md there.

@gziolo
Copy link
Member

gziolo commented Jan 29, 2019

@mkaz and @nosolosw - given your experience with tutoruals, would you mind to help with this one? Maybe it should be divided into section to make it easier to grep.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

See above

@oandregal
Copy link
Member

I see this is a README of a component, not a tutorial. For these, I like them short and API driven. We could use this README as a testbed for the doc generator package I'm working on at #13329 If I check out that branch, then execute:

node packages/docgen/src/cli.js packages/editor/src/components/colors/index.js

a README with the external API of the component is generated. It needs a bit more work to be ready, but not much!

@jorgefilipecosta would you have the time to pair with me until we find the right output/API for the docgen package so it's useful to auto-generate README files like this? I think having the doc generator ready will help us document components a lot faster than we are now.

@gziolo
Copy link
Member

gziolo commented Jan 29, 2019

I love this idea of having docs for components controlled by some automation ❤️

@mkaz
Copy link
Member

mkaz commented Jan 29, 2019

I like the idea of having the documentation with the components but having enough code examples so that is clear what to do. This may require linking or referencing an existing tutorial, so the assumptions of what the user needs to know are clear and discoverable.

Also, I don't mind the longer documentation, I wish I didn't break up the tutorials into so many steps (different pages) I think it might be easier as one long page. Though this is might be more personal preference than anything.

@chrisvanpatten
Copy link
Contributor

After a quick re-read, I agree that I think this is a bit verbose right now and maybe not super clear as a README.

I think a good approach would be to have a README that's more straightforward and terse, with simple examples, and expand on them in a tutorial.

Right now this is almost like a high level design document explaining how it all works, which is interesting but not especially practical.

@jorgefilipecosta
Copy link
Member Author

@jorgefilipecosta would you have the time to pair with me until we find the right output/API for the docgen package so it's useful to auto-generate README files like this? I think having the doc generator ready will help us document components a lot faster than we are now.

Hi @nosolosw, I can dedicate some time to pair with you and move this forward. I would love to help get some automation to generate docs. Thank you for your availability.

Hi @mkaz and @chrisvanpatten thank you for your feedback, I will try to apply some rewrites to get this in better shape.

@gziolo gziolo added this to the Documentation & Handbook milestone Feb 1, 2019
@youknowriad youknowriad removed this from the Documentation & Handbook milestone Mar 18, 2019
@aduth
Copy link
Member

aduth commented Apr 24, 2019

What's the status of this pull request? Given prior comments about rewriting the documentation toward practical examples, would it be fair to close this and open a new one when that comes to fruition?

@aduth aduth added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Apr 24, 2019
@jorgefilipecosta
Copy link
Member Author

What's the status of this pull request? Given prior comments about rewriting the documentation toward practical examples, would it be fair to close this and open a new one when that comes to fruition?

Yes, I guess that's the best path to follow.

@jorgefilipecosta jorgefilipecosta deleted the add/withColors-documentation-and-sample branch April 29, 2019 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. [Type] Developer Documentation Documentation for developers [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants