Skip to content
This repository has been archived by the owner on May 7, 2021. It is now read-only.

fix(inlineinput): float value validation for user input #2816

Merged
merged 8 commits into from
Dec 13, 2018
Merged

fix(inlineinput): float value validation for user input #2816

merged 8 commits into from
Dec 13, 2018

Conversation

abhinandan13jan
Copy link
Contributor

@abhinandan13jan abhinandan13jan commented Dec 6, 2018 •
edited
Loading

Note: If there are pending changes to the PR, prefix the PR title with "WIP" and add the label "DO NOT MERGE"

Mandatory

Optional

  • Is the documentation Included?

  • Are the Release Notes included?

  • @mention(s) to expected reviewer(s) included

@alien-ike
Copy link
Collaborator

alien-ike commented Dec 6, 2018 •
edited
Loading

Ike Plugins (test-keeper)

Thank you @abhinandan13jan for this contribution!

It seems that this PR already contains some added or changed tests. Good job!

Your plugin configuration is stored in the file.

@alien-ike
Copy link
Collaborator

Hey @abhinandan13jan! It seems you tried to trigger /ok-without-tests command, but this will not have any effect due to insufficient permission. You have to be admin or requested reviewer or pull request approver, but not pull request creator for this command to take an effect.

@abhinandan13jan abhinandan13jan changed the title Float field validation in inlineinput [WIP]Float field validation in inlineinput Dec 6, 2018
@centos-ci
Copy link
Collaborator

@abhinandan13jan Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 and visit http://localhost:5000 to access it.

@abhinandan13jan
Copy link
Contributor Author

Added test cases

@abhinandan13jan abhinandan13jan changed the title [WIP]Float field validation in inlineinput Float field validation in inlineinput Dec 6, 2018
@nimishamukherjee
Copy link
Collaborator

@abhinandan13jan please add a reference to the issue that you are fixing.

joshuawilson
joshuawilson previously approved these changes Dec 6, 2018
View reviewed changes
@joshuawilson
Copy link
Contributor

[test]

@centos-ci
Copy link
Collaborator

@abhinandan13jan Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 and visit http://localhost:5000 to access it.

@joshuawilson
Copy link
Contributor

@abhinandan13jan please use "semantic release" syntax for the git commit messages. You can find it in the README.(or google it)

@centos-ci
Copy link
Collaborator

@abhinandan13jan Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 and visit http://localhost:5000 to access it.

@joshuawilson joshuawilson dismissed their stale review December 6, 2018 18:58

I hit the wrong button

@abhinandan13jan abhinandan13jan changed the title Float field validation in inlineinput [WIP]Float field validation in inlineinput Dec 7, 2018
@abhinandan13jan abhinandan13jan changed the title [WIP]Float field validation in inlineinput [WIP] Float field validation in inlineinput Dec 7, 2018
@abhinandan13jan abhinandan13jan changed the title [WIP] Float field validation in inlineinput Float field validation in inlineinput Dec 7, 2018
@abhinandan13jan abhinandan13jan changed the title Float field validation in inlineinput fix(inlineinput): float field validation for user input Dec 7, 2018
@abhinandan13jan abhinandan13jan changed the title fix(inlineinput): float field validation for user input fix(inline-input): float field validation for user input Dec 7, 2018
@abhinandan13jan abhinandan13jan changed the title fix(inline-input): float field validation for user input fix(inlineinput): float field validation for user input Dec 7, 2018
@abhinandan13jan abhinandan13jan changed the title fix(inlineinput): float field validation for user input fix(inlineinput): float value validation for user input Dec 7, 2018
@centos-ci
Copy link
Collaborator

@abhinandan13jan Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 and visit http://localhost:5000 to access it.

sahil143
sahil143 suggested changes Dec 7, 2018
View reviewed changes
Copy link
Contributor

@sahil143 sahil143 left a comment

Choose a reason for hiding this comment

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

When I press enter or click the blue check button with an invalid value in textarea, focus is lost from the input.

src/app/widgets/inlineinput/inlineinput.component.ts
validateValue(value) {
if (this.type === 'integer') {
return /^\d+$/.test(value);
}
if (this.type === 'float') {
return /^-?\d*(\.\d+)?$/.test(value);
return this.isFloat(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

@abhinandan13jan This method is still failing because value is of type string. value needs to be parsed first for this to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sahil143 I was trying not to alter any preexisting functionalities. Now, I've added a .focus() method for the validation failure case. Can you please check again if it looks ok now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshuawilson I've updated using sematic release format

@abhinandan13jan abhinandan13jan changed the title fix(inlineinput): float value validation for user input [WIP] fix(inlineinput): float value validation for user input Dec 8, 2018
@abhinandan13jan abhinandan13jan changed the title [WIP] fix(inlineinput): float value validation for user input fix(inlineinput): float value validation for user input Dec 10, 2018
@sahil143
Copy link
Contributor

[test]

@sahil143
Copy link
Contributor

Looks good to me. Thanks, @abhinandan13jan.

@centos-ci
Copy link
Collaborator

@abhinandan13jan Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 and visit http://localhost:5000 to access it.

@centos-ci
Copy link
Collaborator

@abhinandan13jan Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2816 and visit http://localhost:5000 to access it.

@joshuawilson joshuawilson merged commit ca3a7d5 into fabric8-ui:master Dec 13, 2018
@abhinandan13jan abhinandan13jan mentioned this pull request Dec 14, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sahil143 sahil143 sahil143 approved these changes

@joshuawilson joshuawilson joshuawilson left review comments

@nimishamukherjee nimishamukherjee Awaiting requested review from nimishamukherjee

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@abhinandan13jan @alien-ike @centos-ci @nimishamukherjee @joshuawilson @sahil143