8000 [Form] Add the html5 option to ColorType to validate the input by fancyweb · Pull Request #36302 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Form] Add the html5 option to ColorType to validate the input #36302

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 5, 2020
Merged

[Form] Add the html5 option to ColorType to validate the input #36302

merged 1 commit into from
May 5, 2020

Conversation

fancyweb
Copy link
Contributor
@fancyweb fancyweb commented Apr 1, 2020
Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets no
License MIT
Doc PR TODO

Continuation of #35626.

I'm resubmitting the initial implementation, this time in the Form component.

This Color constraint is dedicated to the HTML5 input type="color".

@fancyweb fancyweb requested a review from xabbuh as a code owner April 1, 2020 15:28
@fancyweb fancyweb mentioned this pull request Apr 1, 2020
Closed
HeahDude
HeahDude reviewed Apr 1, 2020
View reviewed changes
Copy link
Contributor
@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

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

What about a strict_html5 option in a ColorValidatorTypeExtension for the ColorType that adds it with a normalizer automatically?

@stof
Copy link
Member
stof commented Apr 1, 2020

Constraints should not be implemented in the Form component IMO.

@HeahDude
Copy link
Contributor
HeahDude commented Apr 1, 2020

So a solution could be to use the normalizer to add a Regex constraint when the option is true.

@HeahDude
Copy link
Contributor
HeahDude commented Apr 1, 2020

So a solution could be to use the normalizer to add a Regex constraint when the option is true.

But then we should not require the Validator component for such option, add do it in the ColorType directly using a preg_match in a listener instead.

@xabbuh
Copy link
Member
xabbuh commented Apr 2, 2020

I agree, this could simply be part of the ColorType. For consistency with other form types I suggest to add an html5 option which renders an HTML5 input="type" widget and performs these validation rules.

@nicolas-grekas nicolas-grekas added this to the next milestone Apr 3, 2020
@fancyweb fancyweb force-pushed the form-color-constraint branch 2 times, most recently from 31b82a2 to 0f352fb Compare April 8, 2020 10:20
@fancyweb fancyweb changed the title [Form] Add the color constraint [Form] Add the html5 option to ColorType to validate the input Apr 8, 2020
@fancyweb
Copy link
Contributor Author
fancyweb commented Apr 8, 2020 •
edited
Loading

I took your ideas into consideration. That's kind of out of my initial use case but it's still going to be useful.

which renders an HTML5 input="type" widget

AFAIK, that's already the case.

@fancyweb
Copy link
Contributor Author
fancyweb commented Apr 8, 2020

do it in the ColorType directly using a preg_match in a listener instead.

Should we however add a RegExp constraint (and skip the listener) if the Validator component is there?

@fancyweb fancyweb force-pushed the form-color-constraint branch from 0f352fb to caead2d Compare April 8, 2020 12:21
@fancyweb fancyweb force-pushed the form-color-constraint branch 2 times, most recently from 889c936 to 56fd5d5 Compare April 14, 2020 12:11
@fancyweb fancyweb force-pushed the form-color-constraint branch from 56fd5d5 to 454b6ff Compare April 16, 2020 15:25
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot
Copy link
Member
fabpot commented May 5, 2020

Thank you @fancyweb.

@fabpot fabpot mentioned this pull request May 5, 2020
Closed
@fabpot fabpot merged commit 341ea45 into symfony:master May 5, 2020
@fancyweb fancyweb deleted the form-color-constraint branch May 5, 2020 06:52
@fabpot fabpot mentioned this pull request May 5, 2020
Merged
@mhujer mhujer mentioned this pull request May 28, 2020
Merged
@YaFou YaFou mentioned this pull request Jun 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@stof stof stof left review comments

@HeahDude HeahDude HeahDude requested changes

@fabpot fabpot fabpot approved these changes

@xabbuh xabbuh xabbuh approved these changes

Assignees
No one assigned
Labels
Feature Form Status: Reviewed
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
7 participants
@fancyweb @stof @HeahDude @xabbuh @fabpot @nicolas-grekas @carsonbot
0