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

Added descriptions for AutoComponents #659

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MillanWangGadget
Copy link
Contributor

  • UPDATE
    • Added descriptions for AutoComponents exported from the @gadgetinc/react package
    • I left out many of the optional params in the descriptions as I fear they will become foot-guns for the AI
      • Ex: Many AutoInputs re-export onChange from the base Polaris component.
        • When this is overwritten, changing the input value won't include the value in the form submission unless there is some extra code to connect it to the overall form state.
        • If a user gets something like this from the AI, it will be a tough "where is my form value?" bug to fix

@MillanWangGadget MillanWangGadget force-pushed the mill/AddAutoComponentDesriptions branch 3 times, most recently from 378c49a to 70fdb0f Compare October 18, 2024 22:24
@MillanWangGadget MillanWangGadget force-pushed the mill/AddAutoComponentDesriptions branch from 70fdb0f to b3c0665 Compare November 27, 2024 22:39
@greenlemur
Copy link

@MillanWangGadget this has apparently been sitting in my github for some time lol.

Seems super useful for whats about to ship, no? Still need me to review or wanna rebase and rethink it? IMO we should add these descriptions for sure.

@MillanWangGadget
Copy link
Contributor Author

@MillanWangGadget this has apparently been sitting in my github for some time lol.

Seems super useful for whats about to ship, no? Still need me to review or wanna rebase and rethink it? IMO we should add these descriptions for sure.

@greenlemur Yeah I can rebase now. I'll need to add some new descriptions for the new relationship forms, but otherwise, things are gonna generally be the same

@MillanWangGadget MillanWangGadget force-pushed the mill/AddAutoComponentDesriptions branch from b3c0665 to e69cd5f Compare March 11, 2025 02:59
@MillanWangGadget MillanWangGadget force-pushed the mill/AddAutoComponentDesriptions branch from e69cd5f to 20feb53 Compare March 11, 2025 15:38
@greenlemur
Copy link

@MillanWangGadget this has apparently been sitting in my github for some time lol.
Seems super useful for whats about to ship, no? Still need me to review or wanna rebase and rethink it? IMO we should add these descriptions for sure.

@greenlemur Yeah I can rebase now. I'll need to add some new descriptions for the new relationship forms, but otherwise, things are gonna generally be the same

Maybe it makes sense for me to wait until after autocomponents shadcn update and go through every single one when ready? Also, Im just gonna review the content portion so for the other stuff, you might need a different reviewer

@MillanWangGadget
Copy link
Contributor Author

@MillanWangGadget this has apparently been sitting in my github for some time lol.
Seems super useful for whats about to ship, no? Still need me to review or wanna rebase and rethink it? IMO we should add these descriptions for sure.

@greenlemur Yeah I can rebase now. I'll need to add some new descriptions for the new relationship forms, but otherwise, things are gonna generally be the same

Maybe it makes sense for me to wait until after autocomponents shadcn update and go through every single one when ready? Also, Im just gonna review the content portion so for the other stuff, you might need a different reviewer

I've updated the Shadcn side to have the same component prop types. Having the component comments with the code snip examples is gonna be a bigger challenge because of how those components are made by passing in user components from their Gadget apps. I think those docs comments may need to be added on the Gadget monorepo side and included in user code by default.

As for the content of the Shadcn component docs comments, I think they will be the effectively the same as the ones here for Polaris. I think this is good to review now

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.

2 participants