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

Feature (v3): Add Field properties type #760

Closed
hmvp opened this issue Jul 2, 2020 · 3 comments
Closed

Feature (v3): Add Field properties type #760

hmvp opened this issue Jul 2, 2020 · 3 comments
Assignees
Labels
Type: Feature New features and feature requests
Milestone

Comments

@hmvp
Copy link

hmvp commented Jul 2, 2020

In our project we defined this:

export type FieldProps<Value, Extra = {}, InputElementType = HTMLInputElement, ElementType = HTMLDivElement> = Override<
    HTMLProps<ElementType>,
    GuaranteedProps<Value> & {
        inputRef?: Ref<InputElementType>;
    } & Extra
>;

Which lets met define the properties for most custom components like this

export type CustomFieldProps = FieldProps<string>;

or for more complex Fields with extra props:

export type SelectFieldProps = FieldProps<
    string | string[],
    { allowedValues?: string[]; checkboxes?: boolean; transform?(value: string): string },
    HTMLSelectElement,
    HTMLSelectElement,
>;

It might be a good idea to add this to uniforms. This makes it clearer which props are specific for the component and which are generic

BTW: I was a bit suprised that inputRef is not part of GuaranteedProps is that intentional?

@radekmie radekmie self-assigned this Jul 4, 2020
@radekmie radekmie added the Type: Feature New features and feature requests label Jul 4, 2020
@radekmie radekmie added this to Needs triage in Open Source (migrated) via automation Jul 4, 2020
@radekmie radekmie added this to the v3.0 milestone Jul 4, 2020
@radekmie
Copy link
Contributor

radekmie commented Jul 4, 2020

Hi @hmvp. Indeed, we've noticed that the usage of Override is quite the same everywhere and could be deduplicated. Thanks for your suggestion, we'll definitely make use of it.

About the inputRef: no, it's not a guaranteed prop. For example, how should it behave on ErrorField? Also, we've already discussed internally that maybe we'll drop inputRef completely and switch to forwardRef. If you have any feedback on that, do put it here.

Also, we're really glad that you are using v3 for such a long time and provide us with really insightful feedback!

@radekmie radekmie moved this from Needs triage to To do in Open Source (migrated) Jul 4, 2020
@hmvp
Copy link
Author

hmvp commented Jul 6, 2020

Hmm It seems there are three types op props: The guaranteed ones, the specific for the component one and generic but not Guaranteed props like inputRef which (often) might not be set but still should be present on the component to be consistent and flexible in use..

No opinion on inputRef I added everywhere because unstyled did but haven't really used it.. However I do see a use case for it since I noticed that components sometimes can have a lot of "distance" between the outer element (where you typically put the filteredDomProps) and the the actual input component (which you might want to manipulate through code)

@radekmie
Copy link
Contributor

radekmie commented Jul 6, 2020

By removing the inputRef I meant switching to forwardRef and as a result using ref prop, not inputRef (there's no point in having a ref to the current field as it's a functional component).

If possible, I'd really appreciate your review on #764, @hmvp.

@radekmie radekmie moved this from To do to In progress in Open Source (migrated) Jul 8, 2020
radekmie added a commit that referenced this issue Jul 15, 2020
Open Source (migrated) automation moved this from In progress to Closed Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

No branches or pull requests

2 participants