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

Integer field type inconsistency #2223

Closed
2 tasks done
ri0ter opened this issue Feb 7, 2021 · 4 comments
Closed
2 tasks done

Integer field type inconsistency #2223

ri0ter opened this issue Feb 7, 2021 · 4 comments
Labels

Comments

@ri0ter
Copy link

ri0ter commented Feb 7, 2021

Prerequisites

  • I have read the documentation;
  • In the case of a bug report, I understand that providing a SSCCE example is tremendously useful to the maintainers.

Description

Fields across different themes are inconsistent. For schema { "type": "integer"} the form is displaying the field as input[type="number"] input in both core and antD themes, but in all other themes it has input[type="integer"] (on the DOM level) - which is not valid type. According to the widgets documentation:

By default, a regular input[type=text] element is used.
Which implies that both default and antD themes are wrong in it's behavior.

Expected behavior

Fields should behave like described in documentation

Version

v2.4.2

Maybe for now it would be enough to fix all the fields which use invalid type and then restore the consistency with a major release?
Also, wouldn't it be easier to maintain the code if the logic for displaying field would be more explicit/strict, so that StringField wouldn't be responsible for displaying number/integer by default?

@epicfaace I can handle that issue, what do you think about the solution?

@epicfaace
Copy link
Member

Ah, I see. Yes, I think your solution works, to fix invalid types for now.

If you'd like, you could make another PR that restores the consistency (and label it breaking-change) so we only add it in 3.x. I wonder if the more natural change for consistency would actually be to change the default behavior (and document this behavior) so that it uses input[type="number"] for all themes. What do you think?

Also, wouldn't it be easier to maintain the code if the logic for displaying field would be more explicit/strict, so that StringField wouldn't be responsible for displaying number/integer by default?

Perhaps, I'm interested in hearing what you think about your thoughts here -- how do you think this could be accomplished?

@epicfaace
Copy link
Member

Related PR: #2222

@newt10
Copy link
Collaborator

newt10 commented Sep 3, 2021

Ah, I see. Yes, I think your solution works, to fix invalid types for now.

If you'd like, you could make another PR that restores the consistency (and label it breaking-change) so we only add it in 3.x. I wonder if the more natural change for consistency would actually be to change the default behavior (and document this behavior) so that it uses input[type="number"] for all themes. What do you think?

Also, wouldn't it be easier to maintain the code if the logic for displaying field would be more explicit/strict, so that StringField wouldn't be responsible for displaying number/integer by default?

Perhaps, I'm interested in hearing what you think about your thoughts here -- how do you think this could be accomplished?

It seems to me that from a consistency perspective, it would make more sense to keep the default behavior, update documentation and other themes.

@newt10 newt10 added the core label Sep 3, 2021
@heath-freenome
Copy link
Member

Fixed in the v5 beta, see the 5.x migration guide

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants