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

Addon-docs: Change the way PropType used by ArgTable are generated for Flow types #12589

Closed

Conversation

phated
Copy link
Contributor

@phated phated commented Sep 27, 2020

Issue: N/A
Requires: #12588

What I did

I changed the way Flow types were converted into PropType because ArgsTable expects a string for summary and details, but Flow types passed to the converter will contain rich type information that is often lost when just using the raw value (as was done previously).

The way the generation is done, all the previous tests that just used raw values are still passing. Additionally, I added a bunch of tests around more specific types of generation.

It might also make sense to have a discussion around whether ArgsTable should receive the rich type information produced by react-docgen instead of only operating on string values. I think that would resolve issues like #12587 in a cleaner manner.

How to test

  • Is this testable with Jest or Chromatic screenshots? Yep, added a bunch of jest snapshots.
  • Does this need a new example in the kitchen sink apps? No
  • Does this need an update to the documentation? No

If your answer is yes to any of these, please make sure to include it in your PR.

@phated phated requested review from koop and shilman September 27, 2020 02:49
@shilman shilman self-assigned this Oct 3, 2020
@shilman shilman added this to the 6.1 docs milestone Oct 3, 2020
@shilman shilman added the flow label Oct 3, 2020
@shilman shilman changed the title Addon-doc: Change the way PropType used by ArgTable are generated for Flow types Addon-docs: Change the way PropType used by ArgTable are generated for Flow types Oct 3, 2020
@shilman shilman added the maintenance User-facing maintenance tasks label Oct 3, 2020
@shilman shilman modified the milestones: 6.1 docs, 6.2 docs Nov 24, 2020
@shilman
Copy link
Member

shilman commented Dec 1, 2020

Seeing what looks like a formatting regression in this PR:

ZoomAnnotationContentWnd

Versus what I'm seeing in next:

ZoomAnnotationContentWnd

(NOTE: I updated this PR from next just in case it was something in the meantime)

@phated can you possibly:

  1. comment on whether this was intended and if not, fix it
  2. add a story that captures this change in chromatic

Thanks and sorry for the slow slow response time on this 🙈

@phated
Copy link
Contributor Author

phated commented Dec 1, 2020

Very curious! There was no formatting when I originally worked on this. I probably need some time to investigate where that is being tripped up.

@stale
Copy link

stale bot commented Dec 25, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Dec 25, 2020
@stale stale bot removed the inactive label Jan 28, 2021
@shilman shilman removed this from the 6.2 docs milestone Jun 8, 2021
@shilman
Copy link
Member

shilman commented Jul 12, 2021

closing this for now. please feel free to reopen if you need it!

@shilman shilman closed this Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants