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

docs(Combobox): update dev docs; add mdx docs #6734

Merged
merged 11 commits into from
Sep 28, 2020

Conversation

dakahn
Copy link
Contributor

@dakahn dakahn commented Aug 26, 2020

Closes #6629

Updates the dev docs using our new template. Adds MDX file and relevant stories.

Getting it up here so we can start hashing out what this should look like

Testing/reviewing

Make sure all functionality we want to demonstrate is accounted for and working correctly 👍

@dakahn dakahn requested a review from a team as a code owner August 26, 2020 23:32
@netlify
Copy link

netlify bot commented Aug 26, 2020

Deploy preview for carbon-elements ready!

Built with commit aae5638

https://deploy-preview-6734--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Aug 26, 2020

Deploy preview for carbon-components-react ready!

Built with commit aae5638

https://deploy-preview-6734--carbon-components-react.netlify.app

@netlify
Copy link

netlify bot commented Aug 26, 2020

Deploy preview for carbon-elements ready!

Built with commit 877aeff

https://deploy-preview-6734--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Aug 27, 2020

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit 877aeff

https://deploy-preview-6734--carbon-components-react.netlify.app

- [Component API](#component-api)
- [Feedback](#feedback)

## Overview
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be able to use the same strategy as TJ did over in Dropdown for use-cases / component API: https://github.com/carbon-design-system/carbon/blob/master/packages/react/src/components/Dropdown/Dropdown.mdx

Some other examples could be:

  • Skeleton
  • Controlled vs uncontrolled
  • Invalid states
  • Label guidance (how to label it so that it's accessible)

@alisonjoseph alisonjoseph mentioned this pull request Sep 4, 2020
36 tasks
@joshblack
Copy link
Contributor

bump @dakahn let me know when a good time to re-review is for this! 👀

@dakahn
Copy link
Contributor Author

dakahn commented Sep 18, 2020

@joshblack I added some more detail on some pitfall props (a couple you mentioned). Didn't see a skeleton for this component though, but I could just be being daft

@tw15egan
Copy link
Member

Do we think it would be helpful to add guidance on how to use some of the other more complex props like shouldFilterItem?

If we don't want to duplicate documentation for props like initialSelectedItem, we should at least link to other components with similar props so that we have more thorough examples

@dakahn
Copy link
Contributor Author

dakahn commented Sep 18, 2020

I'm not sure how to distinguish which props are complex to use? It could be that I'm just used to looking at these things but when it comes to docs like this
image

It just doesn't seem to provide that much additive value. If we're just going to write a quick sentence and show and example of every prop being used in context then that's cool, but there doesn't seem to be anything particularly complex about that prop, no?

@tw15egan
Copy link
Member

tw15egan commented Sep 21, 2020

I think it's more of along the lines (for me) that it doesn't take much effort to add in that extra value for someone that may need it but will exponentially reduce any question we get asked about that prop in the future. We seem to get a lot of questions from newer React developers, so I don't think it hurts to be too explicit.

I would just like to err on the side of "too much documentation" rather than having to go back and add more in later, which inevitably won't happen.

@joshblack
Copy link
Contributor

@tw15egan @dakahn would it help if we made a list of things to document for ComboBox in addition to what's here and we could make an issue to track it and get what exists merged in? Or would it be better to add the examples to this PR and try and get it in tomorrow/Thursday?

@tw15egan
Copy link
Member

I'm fine either way 👍

@joshblack
Copy link
Contributor

bump @dakahn do you have a preference?

@dakahn
Copy link
Contributor Author

dakahn commented Sep 23, 2020

I'll throw a few more examples in like what i see in TJ's pr and we can call it good

For more information, checkout the Downshift prop
[documentation](https://www.downshift-js.com/downshift#props-used-in-examples)

## Placeholders and Labeling
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Placeholders and Labeling
## Placeholders and labeling

Copy link
Contributor

@joshblack joshblack left a comment

Choose a reason for hiding this comment

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

Looks great!

## Table of Contents

<!-- START doctoc generated TOC please keep comment here to allow auto update -->

- [Overview](#overview)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it help to add the new sections to the TOC too or nah?

@joshblack
Copy link
Contributor

bump @tw15egan if you have a sec today/next week

@dakahn
Copy link
Contributor Author

dakahn commented Sep 25, 2020

oh yeah lemme do that right quick @joshblack

@tw15egan
Copy link
Member

Can we just bump the prop table a bit further up the page? The rest looks good to me 👍

@joshblack joshblack requested review from tw15egan and removed request for tw15egan September 28, 2020 14:29
@kodiakhq kodiakhq bot merged commit 36027c6 into carbon-design-system:master Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update docs for Combobox
3 participants