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

Minor label fixes #141

Merged
merged 3 commits into from Jan 19, 2019

Conversation

Projects
None yet
2 participants
@ypcode
Copy link
Contributor

ypcode commented Jan 4, 2019

Q A
Bug fix? [x]
New feature? [ ]
New sample? [ ]
Related issues? fixes #X, partially #Y, mentioned in #Z

What's in this Pull Request?

A simple label change for the boundaries validation message of PropertyFieldNumber to be more accurate

estruyf and others added some commits Dec 13, 2018

Minor fix in label for PropertyFieldNumber
A more accurate validation message for boundaries validation in PropertyFieldNumber
@estruyf

This comment has been minimized.

Copy link
Collaborator

estruyf commented Jan 19, 2019

Actually, the initial message was correct, as the check was only < lower than or > greater than. Equal was not checked. I have implemented a small code change so that it fixes this.

@estruyf estruyf merged commit 40d05fa into SharePoint:dev Jan 19, 2019

2 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
license/cla All CLA requirements met.

estruyf added a commit that referenced this pull request Jan 19, 2019

@estruyf estruyf added this to the 1.14.0 milestone Jan 19, 2019

@ypcode

This comment has been minimized.

Copy link
Contributor Author

ypcode commented Jan 19, 2019

Then, something should probably be changed in commit 210ad1e 😄

MaximumNumberValidationMessage: "The value should be lower than or equal to:"
...
 if (this.props.maxValue && nrValue >= this.props.maxValue) {
      return `${strings.MaximumNumberValidationMessage} ${this.props.maxValue}`;	      return `${strings.MaximumNumberValidationMessage} ${this.props.maxValue}`;
    }	    }

the message says the value should be "lower or equal" than the maximum value and the check returns this message if the value is greater or equal,
either the message or the check is inconsistent
[EDIT] I mean, if it is accepted to be equal (as the message says, and probably what it should be, min or max value should be inclusive, I think), the check that yields the error message should not compare on equality

I actually PRed this because the message used to say "the value should be lower than X" and I could select X

Am I missing or misunderstanding something ?

@ypcode ypcode deleted the ypcode:patch-1 branch Jan 19, 2019

estruyf added a commit that referenced this pull request Jan 19, 2019

@estruyf

This comment has been minimized.

Copy link
Collaborator

estruyf commented Jan 19, 2019

Got a bit confused with the change and shouldn't have done the code change. Just have reverted that one, so the message should show up correctly.

@estruyf estruyf referenced this pull request Jan 24, 2019

Merged

Merge for 1.14.0 #148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment