User tests: Successful: Unsuccessful:
...e
if list field have both (readonly & disabled) attrs, hidden input must be disabled too
Based on a quick code review, this makes sense.
If we are going to disable hidden input field then it is better to remove it from output at all. The whole logic can be simplified more, I mean less code.
I agree with Achal-Aggarwal. if you too, i apply it to my pr.
As far as I understand it, the hidden field is only there because the <select>
doesn't support the readonly
attribute.
It's a cheap way to achieve sort of a readonly behavior.
However since the select
supports the disabled
attribute, there is not really a need for a hidden field in this case to begin with.
My main question here would be why you would add the disabled and the readonly attribute. Imho disabled implies readonly anyway, but not the other way around (like we have it in code...).
Thus imho we should change the check for readonly to also check if disabled is false, and only add the hidden element if only readonly is set but not disabled.
Labels |
Added:
?
?
|
hmmm. yes your question is correct. why i added disabled and readonly both?!!
I think i do not added both of them to my component's form's xml. in my component i needed to disable or readonly some forms fields based on some fields values in load form function. then some fields may have both of readonly and disabled values together.
now when user save form, in save function i run some function based on fields that user submitted.
i can do very things to fix that input do not have both attributes. but this is more simple.
now your solution is better. i search web and understood that disabled attribute will not work with hidden inputs. (http://www.w3schools.com/tags/att_input_disabled.asp)
sorry for my mistake about this.
now i fixed my pr.
This will always return true: (string) $this->disabled != '1' || (string) $this->disabled != 'true')
You need: (string) $this->disabled != '1' && (string) $this->disabled != 'true')
search web and understood that disabled attribute will not work with hidden inputs.
Ah didn't know that myself but makes perfectly sense if you think about it.
Status | New | ⇒ | Pending |
Labels |
Removed:
?
|
Hi @mhehm
is this issue still valid? As the last comment here was on 8 Jan 2014? Also pull requests to the master branch of this repo are not longer accepted. Can you close this pull request and submit a new one against the staging branch, if this issue still exists?
If there is no Response in a month we will close the PR here.
Thanks for understanding!
closing as per the comment above from @zero-24
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-03-23 17:55:17 |
Closed_By | ⇒ | brianteeman |
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32928&start=0