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

Read only form controls #2177

Closed
shixiedesign opened this issue Mar 21, 2019 · 57 comments
Closed

Read only form controls #2177

shixiedesign opened this issue Mar 21, 2019 · 57 comments
Labels
proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 type: enhancement 💡

Comments

@shixiedesign
Copy link
Contributor

shixiedesign commented Mar 21, 2019

A read only variant of Text input is suggested here: carbon-design-system/carbon-contribution#13 (comment)

The “Disabled” option is not viable, as the label and entered data is not accessible. Proposal is to simply remove the line at the base of the "Filled" version of the component, so that it closely matches with the Disabled component, but passes accessibility.

Overview of component (updated May 13)

  • Read-only variant will be keeping underline and dropping the background of the field. We need the underline for low-vision users to recognize this as a text field. Also loosing the underline makes the element a button-style, which would be confusing.
  • Not moving the text position after all. Keeping text 16px away from left of border helps with preserving the form structure.
  • Regular text cursor and allows text selecting
  • Add not-editable icon, icon in PR Add edit--off icon svg & update metadata.yml #2710
  • Optional tooltip when hovering not-editable icon, to provide reason for the field being read-only
  • Overflow text in the field will show in tooltip on hover
  • Should be considered a state, possible readonly attribute
  • Follow the standard readonly attribute: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#attr-readonly

Design spec (updated May 13)

image

In context explorations:

image
image

@chrisconnors-ibm
Copy link
Contributor

awaiting specification.

@lovemecomputer
Copy link
Contributor

lovemecomputer commented May 9, 2019

i have a couple of questions for implementation:

  • semantic: if this is a unique state, should we think of it as being labelled read-only/readOnly?
  • design: have we decided on design spec for this?

@elizabethsjudd
Copy link
Contributor

From my understanding the implementations should follow the standard readonly attribute: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#attr-readonly

@shixiedesign
Copy link
Contributor Author

Thanks everyone for the thoughts and discussion. I've updated the original issue description with spec, and summarized all the comments here. This issue is now ready for development!

@shixiedesign
Copy link
Contributor Author

shixiedesign commented May 13, 2019

Note

Moving issue back to visual design stage to figure out the following potential changes:

  • Potentially removing not-allowed cursor. Waiting on feedback.
  • Lock icon with tooltip containing reason why field is read-only

@lovemecomputer
Copy link
Contributor

lovemecomputer commented May 13, 2019

should we afford for any kind of status messages/icon/tooltip/etc as part of the pattern for readonly input fields? (typically on forms, if i can't edit something, i want to know why and where i need to go to be able to do so).

i can see the case for product teams being responsible for this, but if we have a particular suggested pattern for how to handle that hinting, we could build it in. (e.g. a lock icon in in the field that can launch a tooltip)

@shixiedesign
Copy link
Contributor Author

Lock icon is a good idea @lovemecomputer ! Lemme try it out...

@lee-chase
Copy link
Member

@lee-chase - for note 2, the problem we would run into if we used the author's recommendations is that all disabled states for carbon would be invalidated based on legibility alone. this was the original reasoning for attempting a new read-only state. the good news is that after working alongside michael gower from the a11y team for several months on this, we are in a safe place in how we are going about designing the new read-only state. i'm guessing the way it is coded will definitely need to keep aspects of the article you mentioned in check however.

I do not have a Sketch license at the moment, so I cannot currently review the latest shared design. However, I have seen a PDF, which may be out of date, that shows an effect very similar to the "light" version of a number of form elements. While the "light" option does not appear in all form components here https://carbondesignsystem.com/components/search/usage/ and it is not functional in some places it does, it does appear as part of the implementation https://react.carbondesignsystem.com/?path=/story/components-textinput--playground&args=light:true appearing as a property on 16 components in @carbon/react v11.

image

@quarryboy
Copy link

@lee-chase very true. the new read-only designs do take that into consideration and also has supporting cursor states that give an additional hint to the user around what is and isn't mutable. one big difference between the light option and read-only is that the light option simply alters the background color to the next value step up (lighter or darker according to the theme - as it is not functionally intended to be seen on the same colored background as the form item), whereas the read-only component has no background color at all.

@lee-chase
Copy link
Member

@lee-chase very true. the new read-only designs do take that into consideration and also has supporting cursor states that give an additional hint to the user around what is and isn't mutable. one big difference between the light option and read-only is that the light option simply alters the background color to the next value step up (lighter or darker according to the theme - as it is not functionally intended to be seen on the same colored background as the form item), whereas the read-only component has no background color at all.

I might be missing something, but from a visual point of view, I'm not sure I see a distinction between having no background and a background that matches the default background.

@quarryboy
Copy link

@lee-chase ah i see where the confusion is. this is a practical UX problem. the "light" background is not intended to match the default background. the intended use of the "light" setting is an alternate setting for when the form item is on a background that is not the default.

meaning: if the white theme is used, then a gray10 background will be applied to the component by default. however, there are times where the component is placed on a gray10 background (rather than white) and contrast is still required to differentiate the background color of the component from its surroundings. the alternate "light" version will instead have a white background to create the appropriate contrast.

does that make sense?

@lee-chase
Copy link
Member

Makes sense @quarryboy, although I suspect as a result that 'light' will have been misused in places.

@quarryboy
Copy link

@lee-chase i feel like we could’ve put money on the line. 😉 It’s almost a guarantee that bit has been misused!

@lee-chase
Copy link
Member

@quarryboy @sstrubberg

NOTE:

  • Carbon form component example http://localhost:3000/?path=/story/components-form--default
  • Date picker, while not a single form component, is built using text boxes and hence could be used on a form. (has no example on form storybook)
  • Dropdown is not a form component and will not be submitted with a form. Unlike a select has an aria role applied to its popup list.
  • Combobox is a form input element with aria-role set as per a default input with list attribute. (has no example on form storybook)
  • Multiselect is not a form component and will not be submitted with a form. Unlike select has an aria role applied to its popup list.
  • Select is a form component that can have a role of ether
  • Slider has an input of type number (should this be a range?) which would get submitted with a form, but has the wrong constructed ID which will confuse. (has no example on form storybook)
  • Time picker, while not a single form component, comprises a text box and two selects and hence could be used on a form. (has no example on form storybook)
  • Toggle is not a form component and will not be submitted with a form.

@lee-chase
Copy link
Member

@quarryboy The toggle button is offset by 1px towards the center in read only (off and on) size 'md'. Is that an accident?
image
image

Also you lose the check mark on the 'sm' size, can you confirm this is deliberate?

@chrisconnors-ibm
Copy link
Contributor

tbh I like it, but if it is intentional I think I'd expect the light source to remain consistent between the two states. :)

@quarryboy
Copy link

@lee-chase i think a call between Mike Eacker and i would solve quite a bit rather than discussing so many threads in here.

regarding the toggle button: this may also be better for an in-person call. are you asking about redesigning the toggles for the enabled state as well as what i'm seeing in the image above is not the read-only design.

@JordanWSmith15
Copy link

@shixiedesign @tay1orjones I see that this is being resolved in the code, but I do not see any guidance on the design site. Can someone point me to the final design guidance for read-only inputs?

@sstrubberg
Copy link
Member

Here is the epic tracking the work @JordanWSmith15. You can also find the pattern work here as a PR, waiting to be approved and merged.

@sstrubberg
Copy link
Member

Closing this issue in favor of the epic I mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 type: enhancement 💡
Projects
Archived in project
Archived in project
Development

No branches or pull requests