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

Add: Form to use addPerson function to show usage of useTreeData #6503

Closed
wants to merge 4 commits into from

Conversation

anmol-fzr
Copy link

@anmol-fzr anmol-fzr commented Jun 6, 2024

Closes #6250

Added usage of addPerson function in collection's useTreeData example

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

@snowystinger
Copy link
Member

You'll need to sign the CLA https://react-spectrum.adobe.com/contribute.html#contributor-license-agreement then close and reopen the PR to get it to take

The tests are failing because main hasn't been merged in, so I've done that for you.

@anmol-fzr anmol-fzr closed this Jun 7, 2024
@anmol-fzr anmol-fzr reopened this Jun 7, 2024
@snowystinger
Copy link
Member

snowystinger commented Jun 16, 2024

GET_BUILD blocked by #6554

@yihuiliao
Copy link
Member

GET_BUILD

</TextField>
<Button type="submit">Submit</Button>
</Form>
</>
```
Copy link
Member

Choose a reason for hiding this comment

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

I appreciate wanting to make this example more complete, but I think we need to be careful not to overwhelm the reader with stuff that isn't super relevant. The point of the example it to show useTreeData, not ListBox, or Form, or TextField, etc. It mirrors the above example for useListData. For more details on using the methods, we have a link to the useTreeData docs. This page is just an overview so I think it's ok to not show everything in detail. So I think either we remove the addPerson function if people think it's confusing and leave the documentation of that to the useTreeData docs, or just leave it since it shows the general usage.

@devongovett devongovett force-pushed the main branch 2 times, most recently from 8cb1a5d to 3013156 Compare July 23, 2024 22:43
@snowystinger
Copy link
Member

Closing, if you're still interested in making this change, we can reopen it. Thanks!

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.

addPerson defined but not used in example
4 participants