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 input fields validation message #437 #521

Merged
merged 8 commits into from
Nov 26, 2024

Conversation

gselderslaghs
Copy link
Member

Proposed changes

This PR is a follow up of PR #438 based on issue #437

Changelog

  • Removed valid from validation criteria, since input is always considered valid as default except if a invalid error status is thrown
  • Added missing invalid CSS selectors styling
  • Fixed styling issue where validation error was rendered in incorrect position by adding a span element in supporting-text wrapper (this might need to update documentation or implement different method)

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to change).

Checklist:

  • I have read the CONTRIBUTING document.
  • My commit messages follows the conventional commit format
  • My change requires a change to the documentation, and updated it accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

wuda-io
wuda-io previously approved these changes Nov 13, 2024
Copy link
Member

@wuda-io wuda-io left a comment

Choose a reason for hiding this comment

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

Ok I see you have added the class validate and invalid. But we should also consider that there ist the pseudo class from CSS :valid and :invalid. We also should implement it via CSS only. I have not read into it in detail, so I dont know how this fits with our features.

@gselderslaghs
Copy link
Member Author

Ok I see you have added the class validate and invalid. But we should also consider that there ist the pseudo class from CSS :valid and :invalid. We also should implement it via CSS only. I have not read into it in detail, so I dont know how this fits with our features.

@wuda-io
I understand your concern, as of my experience the invalid pseudo selector is directly triggered if you put required attribute to an input field
I'm not sure what could be a possible workaround as we don't want all fields marked with invalid highlighting if rendered empty on page load

@gselderslaghs
Copy link
Member Author

@wuda-io I've added a CodePen example that explains this issue. I guess it would be incompatible to work with :valid :invalid pseudo selectors as of my previous mentioned concern

CodePen
Scenario 1
Has invalid peusdo selector, renders directly invalid on page load

Scenario 2
Reversed scenario where default would be invalid but empty would be rendered as valid without the invalid styling as I tried this out as a workaround

Scenario 3
In this scenario I applied the :user-invalid pseudo selector instead of :invalid pseudo selector, this seems to help not rendering the field as invalid on page load and invalidates if the input is incorrect. However, putting an invalid value and removing it from the field does not revert the field styling to normal state, as :user-invalid pseudo class remains, whilst this is not the case on the regular text input field

I don't understand why this is happening, as on Can I Use this selector is listed as supported by all modern browsers, I've also consulted the HTML specification on WHATWG regarding this pseudo selector, from my opinion the :valid :invalid pseudo is currently not functional ready yet to implement it a a required feature in Materialize, as I find quirks in it's functionality, especially on HTML5 input elements

Copy link
Member

@wuda-io wuda-io left a comment

Choose a reason for hiding this comment

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

LGTM

@wuda-io wuda-io merged commit 0f49fee into materializecss:v2-dev Nov 26, 2024
1 check passed
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.

3 participants