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

#4961 - Fix read-only not having any effect on checkbox fields #5164

Merged
merged 1 commit into from Dec 5, 2018

Conversation

atanas-dev
Copy link
Contributor

@atanas-dev atanas-dev commented Sep 28, 2018

Description

Fixes #4961

The "readonly" attribute only prevents the "value" attribute from changing which is not what we want in the case of checkboxes as interacting with a checkbox changes its checked attribute instead which is why we need to do this:

  1. Add disabled="DISABLED" to the checkbox.
  2. As a consequence of 1, even if the checkbox default value is set to 1, the value stored will still be 0 since disabled inputs are not submitted.
  3. To fix 2 we add a hidden input immediately after the checkbox only when the checkbox should be checked.
  4. The hidden input will have the same name and value as the checkbox and will override it since it appears later in the html hierarchy. A bit hacky but avoids name clashes with other fields and the need for special-case back-end code.
Default value Result Stored Value
NONE Checkbox is unchecked and disabled; no hidden input 0
0 Checkbox is unchecked and disabled; no hidden input 0
1 Checkbox is checked and disabled; hidden input is present 1

I decided not to include this lengthy explanation as an inline comment - let me know if I should try to condense it and add it.

Feedback is appreciated.

How Has This Been Tested?

Manually tested a read-only checkbox field with no default value, 0 and 1 as the default value.

Types of changes

Bug fix

ChangeLog

Fix: Checkbox fields not respecting their Make field "Read Only" in UI setting.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

Copy link
Member

@sc0ttkclark sc0ttkclark left a comment

Choose a reason for hiding this comment

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

Good first PR!

@sc0ttkclark sc0ttkclark added the Status: PR > Reviewed and Ready PR has been code reviewed by core developers and is ready for merge (if it passes QA) label Oct 4, 2018
@sc0ttkclark
Copy link
Member

@pglewis can you come back around on this PR branch with the DFV fix too?

@sc0ttkclark sc0ttkclark added Status: In Progress Issue or PR is currently in progress but not yet done and removed Status: PR > Reviewed and Ready PR has been code reviewed by core developers and is ready for merge (if it passes QA) labels Oct 26, 2018
@sc0ttkclark sc0ttkclark added Status: PR > QA pending QA needs to be done and removed Status: PR > QA pending QA needs to be done labels Dec 5, 2018
@jimtrue
Copy link
Contributor

jimtrue commented Dec 5, 2018

QA Tests Passed:

  • Yes/No Checkbox created with default of 0 and Read Only: Wrote 0
  • Yes/No Checkbox created with default of 1 and Read Only: Wrote 1

@jimtrue jimtrue added Status: PR > QA pass QA passed and removed Status: PR > QA pending QA needs to be done labels Dec 5, 2018
@sc0ttkclark sc0ttkclark merged commit f1ca9f3 into pods-framework:2.x Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: In Progress Issue or PR is currently in progress but not yet done Status: PR > QA pass QA passed Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: yes/no fields cannot be made "read-only"
4 participants