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

Placeholder not been applied #152

Closed
vladejs opened this issue Dec 23, 2016 · 13 comments
Closed

Placeholder not been applied #152

vladejs opened this issue Dec 23, 2016 · 13 comments
Assignees
Labels
Type: Bug Bug reports and their fixes

Comments

@vladejs
Copy link

vladejs commented Dec 23, 2016

I have a simple schema and placeholder value is not working properly:

venue:
  type: String
  uniforms:
    placeholder: "doesn't work"

Is maybe a regression?

@radekmie radekmie self-assigned this Dec 24, 2016
@radekmie radekmie added the Type: Bug Bug reports and their fixes label Dec 24, 2016
@radekmie
Copy link
Contributor

Which theme and schema version? I'll take a look at it after a Christmas.

@vladejs
Copy link
Author

vladejs commented Dec 24, 2016

bootstrap3, uniforms v1.6-beta.4. I noticed v1.8 is released, i'll try to update and see if issue persist.
Currently if the schema attribute is a Select (it has allowedValues) it shows me the label as placeholder no matter what.

@cyclops24
Copy link

I test it and it's also not working for me too.

@cyclops24
Copy link

cyclops24 commented Dec 27, 2016

ohhhhhhh @vladejs it's not a bug. You need set placeholder={true} in your form based on doc.
For example it's my form:

<AutoForm schema={LoginSchema} placeholder={true}
       onSubmit={doc => console.log(doc)}>
</AutoForm>

It's part of docs that you can see placeholder is disabled by default.

    // Default placeholder prop for all fields.
    //   By default it's false - set it to true to enable placeholders for the
    //   whole form.
    placeholder={false}

You can read it here: https://github.com/vazco/uniforms/blob/master/API.md#baseform
@radekmie I think just more docs needs about it and it's not a bug man. 😉

@radekmie
Copy link
Contributor

@vladejs: is it working for you?
@cyclops24: I'm glad that somebody is reading API docs :D Maybe you want to submit a PR and add it to the readme?

@vladejs
Copy link
Author

vladejs commented Dec 27, 2016

@cyclops24 , @radekmie It doesn't work and it's a bug, because as the API docs says:

(...) set it to true to enable placeholders for the whole form.

Putting placeholder={true} will set placeholders to all form fields by default, which is not what I want.

I want to put placeholders to fields I choose to. Moreover, even with this prop set to true I can't set custom placeholders to fields, it takes the field's label as placeholder no matter what, for example, the following definition will use the word "Venue" as a placeholder instead of "Select a value":

venue:
    type: String
    allowedValues: ['Home', 'Community', 'Other']
    optional: true
    uniforms:
      placeholder: 'Select a value'

So the expected behaviour should be:

Whether placeholder={true} is set or not on AutoForm, if I set a custom placeholder to a field, it should use it instead of the label.

@radekmie
Copy link
Contributor

How about... Both?

I've found a bug - if you've specified both label and placeholder in your schema, then <AutoForm label placeholder /> results in incorrect placeholder - it's the same as label but it shouldn't.

@vladejs: it's not a bug - it's a consequence of another feature.

Let's describe how it works right now:

label

There is a truthy `label` prop or it is derrivered*?
    There is `label` prop equal to `true` or it is derrivered*?
        Use `label` from schema _(or leave it empty)_.
        Otherwise, use `label` prop.
    Otherwise, there is `label` prop equal to `null`?
        Use `null`.
        Otherwise, empty.

placeholder

There is a truthy `placeholder` prop or it is derrivered*?
    There is `placeholder` prop equal to `true` or it is derrivered*?
        Use `label` from schema _(or leave it empty)_.
        Otherwise, use `placeholder` prop.
    Otherwise, empty.

(please remember, that above solution is incorrect)

I hope, that you see the problem. We can't allow forcing field placeholder through the schema. Schemas defines placeholders but not shows them. If you want to see it, then you have to use placeholder prop on your form or on parent field*.

* Props propagation

Thoughts?


I will deploy this fix tomorrow because my current solution is working, but it grew from 5 lines of logic (described above) to around 30... I'll describe it more in detail tomorrow.

@cyclops24
Copy link

@vladejs I checked what you said and can confirm it's a bug.
@radekmie so based on your comment it's normal and not a bug (it's related to props propagation) or it's a logical bug?

@radekmie
Copy link
Contributor

@cyclops24 There is a bug, that makes the placeholer incorrect (sometimes), but it is showing when it should.

@radekmie
Copy link
Contributor

Fixed. I've also made this logic flat to make it more readable - if you want to know, how it works the source is clear enough.

@cyclops24
Copy link

Thanks @radekmie I checked and it's worked well with custom text when set placeholder={true} in form.

@vladejs
Copy link
Author

vladejs commented Dec 30, 2016

@radekmie , have you tested the example I posted? It still doesn't work for me, I just updated to uniforms-bootstrap3 v1.8.1:

venue:
    type: String
    allowedValues: ['Home', 'Community', 'Other']
    optional: true
    uniforms:
      placeholder: 'Select a value'

On component:

<AutoForm
   schema={FormDailyWeeklyNoteBASchema}
   model={model}
   placeholder={true}
   onSubmit={saveForm}
>
    <AutoFields />
     <SubmitField
        value={model._id ? 'Update Form' : 'Create Form'}
     />
 </AutoForm>

@vladejs
Copy link
Author

vladejs commented Dec 30, 2016

Well, looking to my production environment on modulus it works fine, I think I missed something on my local machine. Anyway, sleep well tonight, I think it's fixed :D

Thanx again and, Happy New Year!.

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

No branches or pull requests

3 participants