extract-react-types icon indicating copy to clipboard operation
extract-react-types copied to clipboard

[WIP] Major API changes

Open declan-warn opened this issue 4 years ago • 7 comments

Seems to work fairly well. Shifted a bit from the original idea.

Needs a little bit of thought about wrappers. E.g. the table layout now requires the user to provide the wrapping table.

Needs docs, etc.

declan-warn avatar Mar 25 '21 06:03 declan-warn

⚠️ No Changeset found

Latest commit: e51a5f6cd78f2df5508052e27aa0e9f5afcca96f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Mar 25 '21 06:03 changeset-bot[bot]

@declan-warn hi! Outsider here 😬

I like the idea of this PR because we want to have a pretty custom layout for our props table.

I was wondering what your thoughts are on using render props rather than passing in a layout - which is still quite restricting.

I.e.

<PropsTable>
  {(props) => do whatever}
</PropsTable>

For example, we want to be able to group and sort our props and have collapsable sections which we wouldn't be able to do using just the custom layout component.

Also I believe generally it would be less maintenance for you guys if you provided one option to do custom things rather than supporting x different versions of a row.

Just food for thought! Let me know what you think :)

davidfloegel avatar Apr 07 '21 05:04 davidfloegel

Hi @davidfloegel,

Thanks for the input! I really like the idea of using render props instead.

Being able to sort and group props is not something I've considered, but it seems completely reasonable.

I'll have a think on what this would look like and bring it up with @danieldelcore.

declan-warn avatar Apr 08 '21 01:04 declan-warn

Hey @declan-warn, amazing!

I'd be super keen to make a PR for this - I've always wanted to contribute to something open source and this I could actually do 😆

Let me know what you guys decide 🙂

davidfloegel avatar Apr 08 '21 05:04 davidfloegel

Amazing!

Sorry I'm still gathering context on this! I'm liking this idea more and more after catching up with Declan this afternoon 😄 I think creating a generic "Layout" component (there's probably a much better name for this haha) in addition to the existing layout components is the way to go.

Something like this:

import { Layout, Prop } from 'pretty-proptypes';


<Layout>
  {(props) => {
    return (
     <div className="my-custom-layout">
       ${props.map(prop => <Prop {...prop} />)}
     </div>
   );
  }
</Layout>

I would really welcome a PR from you @davidfloegel 💯 I think this is a great opportunity to contribute 😄 (Hope that's ok @declan-warn)

Cheers, guys!!

danieldelcore avatar Apr 08 '21 06:04 danieldelcore

Not sure whether "Layout" is the right name, as all it does would be providing the props itself right? (edit: just seen you said this yourself haha)

If you guys want to keep the default layout that you use at atlassian and not make it headless (which probably makes sense), then maybe we could have a separate component for that.

For me, PropsProvider or PropsExplorer which only takes the component you want the props for. Then you could keep your existing code as is and instead wrap that into that new provider.

Alternatively, how do you guys feel about a hook? usePropsExplorer(Component)

I'm only on my phone atm but I will create a draft PR if that is ok with everyone :)

davidfloegel avatar Apr 08 '21 06:04 davidfloegel

@danieldelcore @declan-warn I've had some time before work, so I've gone ahead and created a draft PR. I can't add reviewers for some reason but please do have a look and add yourself :)

davidfloegel avatar Apr 08 '21 08:04 davidfloegel