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 new page to helper-plugin's storybook about useCustomFields hook #15963

Merged
merged 6 commits into from Mar 2, 2023

Conversation

Feranchz
Copy link
Contributor

@Feranchz Feranchz commented Mar 2, 2023

What does it do?

Add a new page to the storybook of the helper plugin that explains how to utilize the useCustomFields custom hook and the functions it returns.

Why is it needed?

To provide more information to developers working on plugins that use custom fields

How to test it?

  • Go to packages/core/helper-plugin and run npm run storybook
  • Go to the useCustomFields doc's page on the hooks section

Or visit the preview

@Feranchz Feranchz added source: core:helper-plugin Source is core/helper-plugin package flag: documentation This PR requires a documentation update labels Mar 2, 2023
@Feranchz Feranchz self-assigned this Mar 2, 2023
@Feranchz Feranchz added source: docs Documentation changes and removed flag: documentation This PR requires a documentation update labels Mar 2, 2023
Copy link
Member

@joshuaellis joshuaellis left a comment

Choose a reason for hiding this comment

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

I think the information hierarchy could be stronger. For instance you've included the return type for get which is fantastic but it's a bit hidden imo. Have you seen how documentation has been written for other hooks in contributor docs? Also this one too.

I'd probably do something like this for the headings:

- useCustomFields
- - Usage
- - - Get a specific custom field
- - - Get all the custom fields
- - Methods
- - `get(uuid: string): CustomField`
- - `getAll(): Record<string, CustomField>`
- - Typescript

I would write the CustomField interface here & the signature of the hook in the Typescript section.

You might think you're repeating yourself by the two usage examples and then writing the methods out but what you're doing is giving readers different ways of accessing the information i.e. they can copy your examples in their app quickly, or learn a bit more about the methods in detail. You also don't need to write copy if you don't want to to accompany the example in great detail. The typescript stuff also is good to explain the return types in a clearer way.

Let me know if any of this doesn't make sense 👍🏼

@Feranchz
Copy link
Contributor Author

Feranchz commented Mar 2, 2023

@joshuaellis I liked the idea, so I tried to apply it. I'm not completely sure if the TypeScript is correct 🤔. Please let me know if you see any issues there

Copy link
Member

@joshuaellis joshuaellis left a comment

Choose a reason for hiding this comment

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

Small TS tweaks but it looks great ⭐

Feranchz and others added 2 commits March 2, 2023 13:24
…ustomFields.stories.mdx

Co-authored-by: Josh <37798644+joshuaellis@users.noreply.github.com>
…ustomFields.stories.mdx

Co-authored-by: Josh <37798644+joshuaellis@users.noreply.github.com>
@joshuaellis joshuaellis added pr: doc This PR contributes to the documentation in this repository (READMEs or Comments) and removed source: docs Documentation changes labels Mar 2, 2023
Copy link
Member

@joshuaellis joshuaellis left a comment

Choose a reason for hiding this comment

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

Great work and initiative to add this ⭐

@Feranchz Feranchz merged commit 6b493cd into main Mar 2, 2023
@Feranchz Feranchz added this to the 4.7.2 milestone Mar 2, 2023
@Feranchz Feranchz deleted the docs/useCustomFields branch March 2, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: doc This PR contributes to the documentation in this repository (READMEs or Comments) source: core:helper-plugin Source is core/helper-plugin package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants