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

Regression with rc.16 #30

Closed
serkandurusoy opened this issue Jun 15, 2016 · 7 comments
Closed

Regression with rc.16 #30

serkandurusoy opened this issue Jun 15, 2016 · 7 comments
Assignees
Labels
Type: Bug Bug reports and their fixes

Comments

@serkandurusoy
Copy link
Contributor

On rc.15

<TextField type="password" name="password">

renders a password field, but on rc.16 this renders a normal input field.

It seems like the changes introduced by commit 6b0ed46 where we set

type={typeof props.type === 'function' ? 'text' : props.type}

instead of

type={props.type || 'text'}

is the root cause.

@radekmie
Copy link
Contributor

Yes, I've made this, because every field already receives a type prop - it's the type from schema (SimpleSchema at least).

type={typeof type === 'string' ? type : 'text'}

should do the job.

@radekmie
Copy link
Contributor

Fixed in 1.0.0-rc.17.

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

macrozone commented Jun 16, 2016

Isn't the type from the schema usually something like String, Number, Object, Array, etc?

I think we should distinguish between both "types" better. E.g. name one "htmlType" (or "inputType" or whatever) and the other "schemaType" or so. It's not obvious which type you currently have, if you are about to design a Form-component. I think its good to be explicit here.

If you would want to force a type in the schema you would probably want to do one of the following:

text: {
  type: String,
  uniforms: {
     htmlType: "password"
  }
}

or

text: {
  type: String,
  uniforms: {
     component: PasswordField
  }
}

(...)
const PasswordField = (props) => <TextField htmlType="password" {...props} />

If htmlType is not set, you would "guess" it from the schemaType:

const htmlType = typeof schemaType === Number ? "number" : "text"

@radekmie
Copy link
Contributor

I thought about renaming schema type to fieldType, because only AutoField uses it.

@serkandurusoy
Copy link
Contributor Author

type is the correct prop for html input elements within the context of JSX therefore instead of renaming it, we should make it so that this is context aware.

Such that;

perhaps the type from the schema gets translated into an internal schemaFieldType property and type from the prop or uniforms key gets translated into an internal htmlInputFieldType

@serkandurusoy
Copy link
Contributor Author

@radekmie it seems rc.16 has not been published to npm yet

@radekmie
Copy link
Contributor

No, 1.0.0-rc17 is already published, but there were no changes in uniforms itself, so it stays in 1.0.0-rc.16.

@radekmie radekmie mentioned this issue Jun 16, 2016
@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

No branches or pull requests

3 participants