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

[Formvalidation] Input array validations should respect each single field on its own #387

Merged
merged 3 commits into from
Jan 22, 2019

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented Jan 12, 2019

Description

If a form uses array inputs by appending brackets to their names ( name="myfield[]") the validation on those fields was always done for the whole field group. Means, if the first field was valid, it validated for the whole group which is wrong.
This behavior is now fixed, so even if multiple input fields have the same name to act as array inputs, the validation is done on each field separately and also the error class is set/removed to each field individually.
@prudho was already providing a PR for that in SUI some time ago, but the individual error display handling was missing there.

Hint: If you ask yourself while reviewing, why i added an internal parameter to the add.prompt()and validate.rule() functions: This is to make sure those functions are still callable as behavior from separate JS (for example: $('.foo').form('validate rule', argumentOne, argumentTwo) so they do not break existing code

Testcase

Before

http://jsfiddle.net/a1cnph4f/
Try to click on submit to invalidate each the form. If any of the multiple fields is invalid it won't get the error class.

After

http://jsfiddle.net/a1cnph4f/1/
Correct behavior now: error class for each invalid field
I also put a normal (single) field into the fiddle to make sure the usual behavior is still working.

Special testcase: Radiobuttons are also working (which need the behavior)
http://jsfiddle.net/47Lysubr/

Screenshots

Before

inputarrays_wrong

After

inputarrays_right

Closes

Semantic-Org/Semantic-UI#6368
Semantic-Org/Semantic-UI#2683
Semantic-Org/Semantic-UI#2688
Semantic-Org/Semantic-UI#5469
Semantic-Org/Semantic-UI#829

y0hami
y0hami previously approved these changes Jan 12, 2019
Copy link
Member

@y0hami y0hami left a comment

Choose a reason for hiding this comment

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

LGTM

@lubber-de
Copy link
Member Author

🤔 I think, I forgot one case, will check and possibly add another commit in 2 hours approx .. Don't merge yet

@y0hami y0hami added the state/on-hold Issues and pull requests which are on hold for any reason label Jan 12, 2019
@lubber-de
Copy link
Member Author

Alright, i had to fix the behavior to work with radio buttons as before
Testcase including radio buttons:
http://jsfiddle.net/47Lysubr/
Please review again

Copy link
Member

@y0hami y0hami left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ColinFrick ColinFrick left a comment

Choose a reason for hiding this comment

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

LGTM

@y0hami y0hami merged commit 5be7315 into fomantic:develop Jan 22, 2019
@lubber-de lubber-de modified the milestones: 2.7.x, 2.7.2 Jan 22, 2019
@lubber-de lubber-de removed the state/awaiting-reviews Pull requests which are waiting for reviews label Jan 22, 2019
@y0hami y0hami mentioned this pull request Feb 4, 2019
@lubber-de lubber-de deleted the fix/6368/validate_input_arrays branch February 5, 2019 11:33
@morkpl
Copy link

morkpl commented Apr 8, 2019

@lubber-de
Problem
inline: true

ice_screenshot_20190408-150041

@lubber-de
Copy link
Member Author

@morkpl Thanks, I'll investigate

@CSDev0
Copy link

CSDev0 commented Oct 10, 2020

maybe i can ask

@lubber-de
Problem
inline: true

ice_screenshot_20190408-150041

this is fixed now? because im using actual release (2.8 ) and i have the same problem. anyway, thanks for such a great framework

@lubber-de
Copy link
Member Author

@morkpl
@Clazherx
Thanks for the reminder
👀 Seems i forgot about this...(because it's not a separate issue but "just" a comment inside a merged PR

@CSDev0
Copy link

CSDev0 commented Oct 26, 2020

@lubber-de thanks to you for the reply.
maybe i will post this bug as a separated issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/javascript Anything involving JavaScript tag/sui-issue Taken from an existing Issue/PR of SUI type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants