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

Readonly inputs don't look like inputs #1129

Closed
w33ble opened this issue Aug 21, 2018 · 4 comments · Fixed by #1188
Closed

Readonly inputs don't look like inputs #1129

w33ble opened this issue Aug 21, 2018 · 4 comments · Fixed by #1188

Comments

@w33ble
Copy link
Contributor

w33ble commented Aug 21, 2018

This applies to search, text, number, textarea, and maybe others. Text input fields with the readOnly prop just look like text. They look odd in the docs:

screenshot 2018-08-21 09 21 58

But it looks especially odd when you try to actually use them in an application.

screenshot 2018-08-21 09 20 49

If it looked more like the disabled input, maybe with darker text and/or lighter background, it would fit a lot better.

screenshot 2018-08-21 09 32 12

screenshot 2018-08-21 09 32 16

@snide
Copy link
Contributor

snide commented Aug 21, 2018

Read only inputs are mostly for displaying text content that matches the padding style of inputs so the spacing is correct (usually a label / input pair in a long form). We use it as substitute for regular text so that things match up.

Specifically we're trying not to signal editability. There's likely a standards argument to make here, but I can't think of a downside to using a disabled input in place of a read only one other than it reading disabled on a screenreader, which in its own way is still likely valid.

@snide
Copy link
Contributor

snide commented Aug 22, 2018

@cchaos or @ryankeairns what do you think about this one?

@ryankeairns
Copy link
Contributor

ryankeairns commented Aug 22, 2018

Digging into this a little bit, there are technical differences. The main difference being that readonly inputs are submitted while disabled are not. I don't think click events will fire on disabled events either, but do on readonly.

Given that, I think we may want to make the read-only style look more like an input box, perhaps a slight variation (?) on the disabled input style.

Likewise, I totally understand the need for labels that share the precise padding/sizing of form elements in order to obtain correct alignment. I don't see label styles called out directly in our EUI library, perhaps we need to add a separate section for those in conjunction with the re-styling of readonly inputs.

@w33ble
Copy link
Contributor Author

w33ble commented Aug 31, 2018

Read only inputs are mostly for displaying text content that matches the padding style of inputs so the spacing is correct (usually a label / input pair in a long form). We use it as substitute for regular text so that things match up.

It doesn't sound like you're using input correctly then. If you want text, display it as text.

I think we may want to make the read-only style look more like an input box, perhaps a slight variation (?) on the disabled input style.

Yeah, read-only inputs still need to look like inputs. If you want an input that looks more like text, add a new input style prop or something. Overloading readonly here is problematic.

cchaos pushed a commit to cchaos/eui that referenced this issue Sep 12, 2018
@cchaos cchaos mentioned this issue Sep 12, 2018
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants