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

Fix initialValue check #164

Merged
merged 1 commit into from
Jan 14, 2017
Merged

Conversation

DimitryDushkin
Copy link
Contributor

Currently there is no way set default value for SelectField, because its value by default is '' (empty string) which is not equal to undefined.

@radekmie radekmie added the Type: Bug Bug reports and their fixes label Jan 14, 2017
@radekmie
Copy link
Contributor

Could make a reproduction? Or at least share schema and/or used form?

@@ -44,7 +44,7 @@ export default function connectField (component, {
return;
}

if (props.value === undefined && props.required) {
if (!props.value && props.required) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be changed - if a field has a falsy value, like false and initialValue equal to true, it will cause a problem. Empty string is caused by ensureValue: true - this flag is used to avoid undefined in HTML inputs.

I'd suggest

- if (props.value === undefined && props.required) {
+ if (props.required && (this.options.ensureValue ? props.value === '' : props.value === undefined)) {

@DimitryDushkin
Copy link
Contributor Author

DimitryDushkin commented Jan 14, 2017

I'm using graphql, so there is no default value in schema. Component looks like following:

<AutoForm
    ref={ ref => formRef = ref }
    schema={ bridge }
    onSubmit={ this.onSubmit }>
    <SelectField
        name='adapterset'
        allowedValues={ adaptersetOptions }
        transform={ getEnumOption.bind(this, sampleEnums.adapterset) }
        initialValue='ILLUMINA_GENERIC'
        required={ true } />
</AutoForm>

Schema:

        input SampleInput {
            adapterset: String!
        }

Your suggestion looks good, I'll rewrite commit.

@radekmie
Copy link
Contributor

  1. OK.
  2. Wait, what?
  3. I've never thought about passing initialValue to the component!
  4. To be honest, that's a very good idea!
  5. I'm really glad to see, that you are using GraphQL with uniforms!
  6. If you are using GraphQL schema, then you can use third parameter of GraphQLBridge (example here).

@DimitryDushkin
Copy link
Contributor Author

DimitryDushkin commented Jan 14, 2017

  1. Oh, I missed that option( I think this better then inlining allowedValues and such in components (separation of concerns and such 😃 ).

I've rewritten commit.

@radekmie radekmie merged commit e03f23a into vazco:master Jan 14, 2017
@DimitryDushkin
Copy link
Contributor Author

BTW graphql intergation works quite good with form! Thank you for this.)
The only "problem" I've faced — warning from webpack of not presence dependency of simpl-schema module. I'll try to send PR on this someday.)

@DimitryDushkin DimitryDushkin deleted the DimitryDushkin-patch-1 branch January 14, 2017 17:48
@radekmie
Copy link
Contributor

I've tried to get rid of it and failed few times so far. If you could handle it or help to do so - I'd love to help.

@Monteth Monteth added this to Closed in Open Source (migrated) Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bug reports and their fixes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants