When I use the #state structure below to turn a text field into read-only depending to a checkbox selection, simply nothing happens.
'states' => array(
'readonly' => array(
':input[name="text_field"]' => array('checked' => TRUE),
),
),
If I use 'disabled' instead of 'readonly' the code works perfectly! But I really need the "readonly" option because I want to save the value filled in the text field before. The "disabled" option doesn't allow to save the field value.
I found a simply solution that solves the problem with few lines: just implement the "readonly" option in the file states.js, the same way it is done with the "disabled" option. This way, readyonly works very well.
| Comment | File | Size | Author |
|---|---|---|---|
| #31 | interdiff_29-31.txt | 664 bytes | nitin shrivastava |
| #31 | 286638-31.patch | 664 bytes | nitin shrivastava |
| #29 | 2866383-29.patch | 614 bytes | Abhisheksingh27 |
| #26 | interdiff_25-26.txt | 1.29 KB | narendra.rajwar27 |
| #26 | 2866383-26.patch | 1.34 KB | narendra.rajwar27 |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !3692
Comments
Comment #2
luizps commentedFollowing the W3C specifications for ready-only (https://www.w3.org/TR/html4/interact/forms.html#h-17.12.2), the solution applies the "readonly" option only to input and textarea fields.
Comment #3
luizps commentedComment #5
luizps commentedComment #7
luizps commentedComment #8
intrafusionI confirm the patch in #2 solves this problem
Comment #9
dev.patrick commentedComment #10
caminadaf commented+1 RTBC
Comment #11
dieterholvoet commentedI updated the patch for the 8.7.x branch.
Comment #13
savel commentedDieterHolvoet, sorry to tell, but you might better don't do it this way.
The issue is for D7 and it was RTBC.
Automated testing system tried to apply your D8 patch to D7 and it failed.
I just put #2 back and it possibly needs to be reviewed again by community to put it to RTBC and wait for commit.
Comment #14
savel commentedSorry, my windows machine has added .txt extension to the patch and I've missed it.
Again, it's just a reroll of #2 wich was RTBC before.
Please, review it and set to RTBC state again.
Comment #15
savel commentedSorry, it happened again.
The patch from #2 as is. It was RTBC before patch for 8.x was added to the thread.
Comment #16
nikolas.tatianenko commentedComment #17
Oscaner commentedFor core 8.7.x
Comment #18
alexpottThis needs to change states.es6.js and compile it to make states.js - see https://www.drupal.org/docs/8/frontend-developer-tools-for-drupal-core
Also thanks for filing this bug report and for fixing it. Bug fixing is very valuable. However in order to commit a bug fix we need an automated to test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal 8 see the following links:
Comment #19
klonosBeen trying to test this (in D7 and Backdrop), but readonly does not seem to have any effect on the browsers. I then did some research and noticed this: https://html.spec.whatwg.org/multipage/input.html#attr-input-readonly
Comment #21
rp7 commentedRe-rolled patch in #17 for 9.1.x.
Comment #25
nikhil_110 commentedRerolled patch #17 for 9.4.x.
Comment #26
narendra.rajwar27Fixing custom commands failure.
Comment #28
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
Did not test but still needs tests.
Comment #29
Abhisheksingh27 commentedAdded Reroll for 10.1.x as #26 patch failed to apply on 10.1.x.
Removed changes of "a/core/misc/states.es6.js" as file doesn't exist.
Comment #30
smustgrave commentedThank you but please read the comments and tags before changing status. Still needs tests
Comment #31
nitin shrivastava commentedfix command failures
Comment #32
nitin shrivastava commentedComment #33
smustgrave commentedThank you for working on this but please see the tags also. This needs tests so this isn't ready for review.
Comment #36
sunlixHey,
I have added
state:readonlywith running Javascript Tests as well.I hope that is properly implemented for a valid core commit. :)
Comment #37
smustgrave commentedTest assertions added cover the change being made.
Ran locally without the fix and got failing assertions
Failed asserting that false is true.Comment #38
nod_Really wanted to commit that but readonly doesn't work on checkboxes, the test need to use an input type that supports the readonly attribute.
The attribute is added correctly and we can see that in the test, it's just that it's not supposed to do anything on radio/checkboxes/color/file/select
Comment #39
sunlix@nod_
Thank you for your feedback. I have done a little bit research on the spec and MDN.
Is far as I understand we have to limit the relvant input options to
text, search, url, tel, email, password, date, month, week, time, datetime-local, and numberor a negated version ofcheckbox, radio, file, range, color, hiddenfor theinputtypes.And for sure adjust the test suite properly. Am I right?
From MDN
Comment #40
nod_I don't think we need to be too precise, if someone uses it on an input that doesn't support readonly it won't break anything specifically.
Comment #41
nod_great, thanks :)
Comment #42
sunlix@_nod
wow, you are great :) Thank you for the proactive obversation.
I just wanted to wait for the tests are green :D I think I got it. So thank you! :)
Just have one follow up question.
The visual representation of the state is nothing special by user agent style.
Should claro & olivero represent the readonly state equal to the disabled state on theme level?
Comment #43
nod_We could open a new issue to address the styling of readonly fields. The styling is out of scope for this issue though.
Comment #44
nod_Comment #45
dwwThe tests are right where they need to be, the additions are clean and follow the existing patterns in the test, are passing now (and as reported by smustgrave, failing when run without the fix).
The fix looks clean and good.
I don’t see any problems with the changes in the MR.
RTBC
thanks!
-Derek
Comment #47
nod_Committed ac64dce and pushed to 10.1.x. Thanks!
Comment #49
nod_Checked in with lauriii, If we consider this a bug (as in: it's expected to be available based on what is possible inside HTML forms), backporting to 10.0.x also because it's better to keep the states API in sync when possible.
Comment #50
luenemann