Skip to content

Make UrlTypes render as url types. #31284

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

Closed
wants to merge 1 commit into from
Closed

Make UrlTypes render as url types. #31284

wants to merge 1 commit into from

Conversation

lmlsna
Copy link

@lmlsna lmlsna commented Apr 27, 2019

Q A
Branch? 4.2 for bug fixes
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #31283 #30635 #29926 #29690
License MIT
Doc PR

Currently you have to declare an explicit option of ["default_protocol" => null] on every UrlType form element or it renders as a type="text". Seems like 2 merges tried to fix this from different ends with one forcing a default protocol and once changing to a text type if there is no default protocol, thus ensuring that url type are always changed to text types unless the above option is also passed, which is very counter-intuitive.

This PR fixes that by no longer changing to text types if default_protocol is not null since we are always forcing a default now.

@ro0NL
Copy link
Contributor

ro0NL commented Apr 27, 2019

if you define a default protocol, you should be able to submit a protocol-less URL. Can you confirm?

perhaps we can improve UX without a default protocol in core:

$resolver->setDefault('default_protocol', 'http');

personally i wouldnt mind deprecating the option either :)

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Apr 27, 2019
@nicolas-grekas
Copy link
Member

You mean that the behavior on 3.4 is good, but you target 4.2 here because of a bad merge?
Can you add a test case please?

@lmlsna
Copy link
Author

lmlsna commented Apr 30, 2019

@ro0NL Okay, I understand now. The type="url" validation requires an explicit protocol and there's no way to insert that when a default protocol is defined, so a type="text" has to be used to allow form submission. That makes sense. My mistake, nevermind 😳.

@lmlsna lmlsna closed this Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants