fix(readonly): make readonly state readable on mobile devices#3719
fix(readonly): make readonly state readable on mobile devices#3719
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds readonly input styling to ensure explicit text color and full opacity on mobile browsers. New host selectors apply the input text color and disable opacity override for readonly and disabled input states, addressing Safari's default styling behavior on iOS. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| :host(.lime-text-field--readonly), | ||
| :host([readonly]) { | ||
| .mdc-text-field--disabled .mdc-text-field__input, | ||
| .mdc-text-field__input[readonly] { | ||
| opacity: 1; | ||
| color: shared_input-select-picker.$input-text-color; | ||
| -webkit-text-fill-color: shared_input-select-picker.$input-text-color; | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this will override the above? (the styles for other types. Can you check?
There was a problem hiding this comment.
Thanks Kia. I checked with browser stack but don't see it's overrides anything. Also this bug appears only in the input-field..
aba4876 to
9abd8b0
Compare
| color: shared_input-select-picker.$input-text-color; | ||
| -webkit-text-fill-color: shared_input-select-picker.$input-text-color; | ||
| } | ||
| } |
There was a problem hiding this comment.
Thanks for checking. But the specificity of this rule is really high. Why do we need to make it so specific?
It feels we could just have something like:
.mdc-text-field__input[readonly] {
// opacity: 1;
// color: shared_input-select-picker.$input-text-color;
-webkit-text-fill-color: shared_input-select-picker.$input-text-color;
}Do we have to nest it in double selectors for the readonly:
:host(.lime-text-field--readonly),
:host([readonly]) {
…
}and even further nested selector classes?
Are you sure we need the opacity and color? If this is an iOS/Safari fix, the -webkit-text-fill-color: should be enough. If all the 3 rules are needed, maybe also consider adding a docs block together with this piece of code, so the next reader knows why this overrides are there. Because they look brutally specific and hard to understand why they should be kept there.
There was a problem hiding this comment.
Yeah, I tested and we can avoid overriding with color
color: shared_input-select-picker.$input-text-color;
but we do need opacity 1
I simplified the nested part and added a doc block
| :host(.lime-text-field--readonly), | ||
| :host([readonly]) { |
There was a problem hiding this comment.
Do we need both of these? I think onl this one is enough:
:host([readonly]) {There was a problem hiding this comment.
yes, that's enough. fixed
39e971e to
5355434
Compare
|
🎉 This PR is included in version 38.29.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix: #3718
Summary by CodeRabbit
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: