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

use :read-only css selector instead [readonly] for consistency #33642

Merged
merged 2 commits into from Apr 17, 2021

Conversation

coharishreddy
Copy link
Contributor

@coharishreddy coharishreddy commented Apr 15, 2021

There are 5 places where [readonly] selector is used. I have replaced with :read-only for consistency.

fixes #33101

There are 5 places where [readonly] selector is used. I have replaced with :read-only for consistency.

fix for twbs#33101
@coharishreddy coharishreddy requested a review from a team as a code owner April 15, 2021 09:13
Copy link
Member

@ffoodd ffoodd left a comment

Choose a reason for hiding this comment

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

Looks good to me since :read-only support is aligned in our browser support.

We may have side effect since it's not targeting the attribute anymore (which one may have used inappropriately on any HTML element) but pseudo-class only applies when it makes sense, so I'd go for it.

@XhmikosR XhmikosR changed the title use :read-only css selector instead [readonly] for consistency use :read-only css selector instead [readonly] for consistency Apr 15, 2021
@mdo mdo added this to Inbox in v5.0.0 via automation Apr 17, 2021
@mdo mdo merged commit 17252bb into twbs:main Apr 17, 2021
v5.0.0 automation moved this from Inbox to Done Apr 17, 2021
@dvorakdominik
Copy link

dvorakdominik commented May 6, 2021

I think :read-only is a problem with select. As you can see in the example below, select has a different background than input type = "text"

https://codepen.io/eter94/pen/jOBOgKd

@ffoodd
Copy link
Member

ffoodd commented May 6, 2021

@eter94 Please open a dedicated issue for us to track, as we won't reopen nor comment a merged PR to fix anything.

Thanks for providing a test case though, that's vert appreciated.

@DannyFeliz
Copy link

@eter94 @ffoodd I opened an issue here #33860

mdo added a commit that referenced this pull request May 13, 2021
XhmikosR added a commit that referenced this pull request May 13, 2021
…tency (#33642)" (#33961)

This reverts commit 17252bb.

Co-authored-by: XhmikosR <xhmikosr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v5.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

v5: Use :read-only instead of [readonly]
5 participants