[TwigBridge] Provide a default non-empty label on empty options to validate HTML5 #48035
Conversation
|
Hey! To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done? Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review. Cheers! Carsonbot |
DocFX
force-pushed
the
patch-5
branch
11 times, most recently
from
4cf75e6
to
c09ab9b
Compare
|
Remaining broken tests (cache & doctrine) seem to have nothing to do with this PR. Not sure I can help with that on that one. |
|
@nicolas-grekas this should be a follow-up to #44464 if I got everything right. :) |
|
Hey! I think @alexander-schranz has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
Oh gosh, you're completely right, I went too fast and forgot some tests XD |
|
I have a HUGE cache problem with Github... When I look at the tests here it says they're broken, even with force-refresh, no cache in browser, and with I go to "actions" page to see details, tests are OK. Can anyone help and tell me what they see? XD |
|
https://github.com/symfony/symfony/actions/runs/3363313786/jobs/5576279353 does not fail for you? |
|
Please find a PR description that tells me what the PR does. The PR description will be used as a change log entry. |
DocFX
changed the title
Sure, edited the title and description! |
|
That's exactly my problem, @xabbuh: this is not my code, and it's not the one from the PR. This job is testing... Something different. I looked at the CI job and couldn't why out why, but I was missing more time to investigate. |
DocFX
force-pushed
the
patch-5
branch
3 times, most recently
from
53d7b5d
to
80ec9fc
Compare
|
(meanwhile, someone fixed the "Integration / Tests (8.1) (pull_request)" job, so this one now passes :) ) |
As described on https://rocketvalidator.com/html-validation/element-option-without-attribute-label-must-not-be-empty, so far this template generates empty options with no value, no content, and no label. This is not a break, but a simple "should".
Not sure how we should recommend handling this, users have the possibility to handle this themselves (https://symfony.com/doc/current/reference/forms/types/choice.html#placeholder), but if they don't we leave a non-stadanrd-compliant output.
The tailwind template is also affected from this, as it extends this one.
I proposed a double-dash default value:
I locally fix this by overriding the default templates, but I thought the default ones could get a fix.
(NB: this is just a new PR for #44464, as previous one was merged in haste and reverted, with tests up to date this time)