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

Fix error messages not being displayed with nested input elements #314

Merged
merged 2 commits into from May 31, 2020

Conversation

ndm2
Copy link
Collaborator

@ndm2 ndm2 commented May 29, 2020

So it's been some time, but there hasn't really been any feedback on how to handle it.

The source of the problem is that the error message element is not a sibling of an (input) element with an is-invalid class. This PR fixes that problem in 3 different places, where the input element is nested in a div:

  1. In input groups
  2. In horizontally aligned checkboxes
  3. In custom file controls

For input groups, Bootstrap now officially suggests to put the validation response as a sibling to an input group with an is-invalid class, so I thought it's safe to go that route, and add the error class to the input group wrapper. Before/After:

input-group-before-after

In horizontally aligned checkboxes, the message (and help text) is moved into the nesting div, which markup wise and visually will unify with default aligned checkboxes. Before/After:

horizontal-checkbox-before-after

For custom file controls I've chosen adding the error class to the wrapper div too, just like for input groups, because a) moving the message element into the wrapper requires some bigger changes with additional templates, and b) it produces a visual glitch as Bootstrap by default restricts the elements height, causing the error message to "overlfow" and being placed over other elements. Before/After:

custom-file-before-after

Problem with message in the wrapper:

custom-file-overflow-problem

A word of warning, none of these fixes are compatible with custom client-side validation, but it didn't work before either, so there's that.

ps. The tests keep growing and growing, and I think I could combine the tooltip and error tests, and also throw in help text usage, that would reduce the clutter a bit, but I'd do that in a followup, I need some rest first, putting these tests together is really tedious.

/cc @ravage84 Not sure, but IIRC you wanted to be notified about this.

ndm2 added 2 commits May 29, 2020 16:25
The error message element is not being displayed when it's not a sibling
of an (input) element with an `is-invalid` class, which is the case for:

* Input groups
* Horizontally aligned checkboxes
* Custom file controls
@codecov
Copy link

codecov bot commented May 29, 2020

Codecov Report

Merging #314 into cake-4-bs-4 will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                Coverage Diff                @@
##             cake-4-bs-4     #314      +/-   ##
=================================================
+ Coverage          89.46%   89.56%   +0.10%     
- Complexity           272      277       +5     
=================================================
  Files                 19       19              
  Lines                816      824       +8     
=================================================
+ Hits                 730      738       +8     
  Misses                86       86              
Impacted Files Coverage Δ Complexity Δ
src/View/Helper/FormHelper.php 98.96% <100.00%> (+0.01%) 96.00 <0.00> (+3.00)
src/View/Widget/InputgroupTrait.php 96.61% <100.00%> (+0.24%) 19.00 <0.00> (+2.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35b83c0...77acb9e. Read the comment docs.

@ADmad ADmad merged commit f3eeb49 into FriendsOfCake:cake-4-bs-4 May 31, 2020
@ravage84
Copy link
Contributor

ravage84 commented Jun 2, 2020

@D4rkMindz please have a look and give feedback if you have any issues.

@ndm2 ndm2 deleted the fix/error-messages branch January 23, 2021 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants