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

Validation error style classes problem #621

Closed
NicolaIsotta opened this issue Jan 19, 2017 · 14 comments
Closed

Validation error style classes problem #621

NicolaIsotta opened this issue Jan 19, 2017 · 14 comments
Assignees
Milestone

Comments

@NicolaIsotta
Copy link
Contributor

NicolaIsotta commented Jan 19, 2017

Sorry to bother you guys, but I'm pretty sure there's a problem with error style classes.
Here's the HTML from your showcase of a input text with failed validation:
input_with_error
The issue is that the has-error class should be added to the form-group div, not to the label or input elements; source: http://getbootstrap.com/css/#forms-control-validation
Also, the bf-error class and its equivalents should be removed, or at least have no default css rules, because they override bs default styles.

@NicolaIsotta NicolaIsotta changed the title Input error style classes problem Validation error style classes problem Jan 19, 2017
@stephanrauh
Copy link
Collaborator

The Bootstrap style looks better, too.

I just wonder how to implement this without breaking backward compatibility. I'm sure many developers rely on the bf-* classes.

@stephanrauh
Copy link
Collaborator

@TheCoder4eu @zhedar @asterd Any thoughts on this? I'd love to collect your feedback before I start implementing something and breaking the applications of several thousand developers.

@TheCoder4eu
Copy link
Owner

@stephanrauh it depends on how they rely on the bf-* classes.
If they rely only on its presence, we just need to keep adding this classes as before removing the associated styles and applying in addition the Bootstrap classes correctly.

@TheCoder4eu
Copy link
Owner

@stephanrauh can we close this issue too?

@TheCoder4eu
Copy link
Owner

@stephanrauh can we close this issue?

@stephanrauh
Copy link
Collaborator

stephanrauh commented Mar 20, 2017

I'd like to keep this ticket open. I think we should support the Bootstrap approach, too.

@sergiowww
Copy link
Contributor

sergiowww commented Mar 21, 2017

I've been hung up on the same problem, the has-error class should be added to the form-group container. Neither the input field nor the field's label should receive feedback classes, it is exactly what @NicolaIsotta said. I've cloned the project and I'll submit a PR as soon as I fix it, I don't know if this issue has been fixed, so please, if anyone have any news about it, keep us updated!
error
!

@stephanrauh
Copy link
Collaborator

OK, go ahead!

For the sake of backward compatibility, please make the error classes configurable:

  • check the parameter legacyErrorClasses of the web.xml:
    BsfUtils.getInitParam("net.bootsfaces.legacy_error_classes");
  • render the old (and wrong) style if the parameter is set to true
  • render the new style if the parameter is empty or false

Thanks in advance!
Stephan

@sergiowww
Copy link
Contributor

Copy that! Thanks!

@sergiowww
Copy link
Contributor

The Pull Request is ready for approval!

@stephanrauh
Copy link
Collaborator

stephanrauh commented Mar 22, 2017

I've merged the PR. Todos left:

  • document the feature
  • double-check if the PR already covers every component able to display error messages

stephanrauh added a commit that referenced this issue Mar 22, 2017
@stephanrauh
Copy link
Collaborator

stephanrauh commented Apr 8, 2017

The feature is still broken with:

  • <b:colorPicker>
  • <b:slider>
  • <b:slider2>

@TheCoder4eu
Copy link
Owner

@stephanrauh , did the commit 2ab0b09 fix also Slider and Slider2?
We can close this issue?

@stephanrauh
Copy link
Collaborator

It solved the bug partially. But it's good enough to close the issue.

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

No branches or pull requests

4 participants