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

fix(FluidTextInput): label overflow scroll #12261

Merged
merged 3 commits into from
Oct 31, 2022

Conversation

jnm2377
Copy link
Contributor

@jnm2377 jnm2377 commented Oct 7, 2022

Closes #12082

Changelog

  • input label should now overflow on scroll

Testing / Reviewing

  • go to the playground story for fluid text input, update the label text to have a really long label that should overflow, check that the label scrolls instead of wraps

@jnm2377 jnm2377 marked this pull request as ready for review October 7, 2022 20:02
@jnm2377 jnm2377 requested a review from a team as a code owner October 7, 2022 20:02
@netlify
Copy link

netlify bot commented Oct 7, 2022

Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit 053e2f4
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/635bf7ca60c2f9000ad1110a
😎 Deploy Preview https://deploy-preview-12261--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Oct 7, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 053e2f4
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/635bf7ca13f5120008f85a4a
😎 Deploy Preview https://deploy-preview-12261--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@aagonzales
Copy link
Member

The scroll bar covers up the label some times and it does fade away eventually. I guess there's probably nothing we can do about that though.

image

@aagonzales
Copy link
Member

Actually, the field text doesn't have the scroll bar either. Is there a way to fix this in the label then?

Oct-11-2022 10-32-38

@tay1orjones
Copy link
Member

@aagonzales Sometimes the scroll bar doesn't fade away depending on the user settings

image

image

I suspect we might see similar problems with the scroll bars in windows.

The textbox scrolling is something the browser does by default. It's possible to hide the scroll bar on the label, but WCAG recommends against it citing usability and accessibility concerns.

It looks like there's a new draft spec for scrollbar-width: thin that would make the scroll bar thinner for cases like this, but it's not available in browsers yet.

All said, I think hiding the scrollbar in this case could be the most beneficial option.

@aagonzales
Copy link
Member

The good thing is, either way, I think label scrolling is a very side edge-case. If we can remove it without causing violations, then lets do it but if we have to keep it then its fine. @jnm2377 @tay1orjones

@tay1orjones
Copy link
Member

@mbgower do you think in this instance it's acceptable to remove the scrollbar?

@mbgower
Copy link

mbgower commented Oct 18, 2022

The primary requirement for WCAG is covered under Reflow.
Basically, the system needs to be able to zoom up to 400% without requiring horizontal scrollbars to make content visible.

So it really depends on how you're handling breakpoints and text resizing. IMO, a text input area is likely an acceptable place to use horizontal scrolling without violating this because the input width is constrained by design -- although I assume Carbon has some guidance saying that where the character count allows more characters than can be displayed, use a text area! I know Carbon does have guidance saying input labels should be short, so I think your design guidance is making this outcome very unlikely.

But you're asking about the opposite of this. 'Can I remove scrolling?' There are a couple of things to note...
Unless the input itself is short and only the label is long, I do not actually think it's going to matter for a keyboard user, because when I enter the input itself, I can enter and edit text, and read it, and just using my cursor keys or home/end to read the content. No "scrolling" is needed. I simply move around in the text.

The place where you create a potential issue if you don't offer scrolling is actually the mouse user. But I believe the interaction workaround would be that they too would click in the input and use arrow keys, or the OS affordance for panning within text, to read the information. If that's acceptable to you folks, I don't think it violates accessibility (which tends to be worried about keyboard more than mouse).

So that only leaves the label as a potential problem. I think I was reading above that the label will line break, rather than scroll? So I'm not sure I understand a scenario where you'd need scrollbars.

@mbgower
Copy link

mbgower commented Oct 20, 2022

I thought up an example... Use of read-only inputs. In such a situation, if you are constraining the horizontal width, you could potentially be in a situation where the user would be unable to see either a very long label or any input text that exceeds the horizontal spacing of the input. @quarryboy, do you have any thoughts on this?

@quarryboy
Copy link

@mbgower would the label not be seen in that instance? i thought it was the opposite of disabled and would be seen/processed for read-only.

i think it's a bit concerning that the label would be more than a few words long. what's the actual use case? it seems like the emphasis should be on putting all that text into a tooltip instead.

@aagonzales out of curiosity, how does this test with browsers that use a two-finger horizontal scroll in conjunction with it's history? e.g.—two-fingers to the left causes you to go "back". seems like it would be very frustrating to accidentally go back in history and have to start the entire form process over again. i saw somethign about violations above? how do violations fit in with this particular use case?

@aagonzales
Copy link
Member

aagonzales commented Oct 20, 2022

I know Carbon does have guidance saying input labels should be short, so I think your design guidance is making this outcome very unlikely.

@mbgower Yes, this is a very rare edge case. I don't expect it to happen often, if at all since labels are pre-defined. I think the most likely instance will be in translation which could trigger a label to scroll (especially with a short input).

We do not want the labels to wrap in the Fluid style because it will effect the container size.

how does this test with browsers that use a two-finger horizontal scroll in conjunction with it's history?

@quarryboy We have not tested it and it hasn't been brought up before. Horizontal scrolls already exist in the default text input along with several other components like data table and code snippets. If horizontal scrolls are triggering violations then that will have consequences across components and we will need to spin up a workstream to address that system wide.

@aagonzales
Copy link
Member

@mbgower What we're really asking is can we hide the scroll bar because the scrollbar when active is covering up the label content.

See #12261 (comment)

@mbgower
Copy link

mbgower commented Oct 20, 2022

@aagonzales it's a question of balancing considerations in the edge case where the label is long. On the one hand, the scrollbar somewhat obscures the label. But in the example you show, it is only obscuring while the user is actively employing it. You can reposition with the scrollbar and then move your mouse away and see the adjusted text position. So, there is a way to see the text. Whereas, if you remove the scrollbars entirely, you seem to be removing a users ability to reposition the label to even see one that is truncated?
If that is a correct summary, then I think it is less risky to leave the scrollbars in place. However, if I'm wrong about removing the scrollbars removing functionality -- if it merely removes the visual cue for scrolling but still allows a user to do it -- then I think hiding the scrollbars is fine.

@tay1orjones
Copy link
Member

if it merely removes the visual cue for scrolling but still allows a user to do it -- then I think hiding the scrollbars is fine.

Yes, we'll only be hiding the visual scrollbar. The ability to scroll will be retained.

Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

The scrollbar on the label is now hidden - the scrolling interaction between both the label and the input is consistent

consistent.scroll.interaction.mov

@kodiakhq kodiakhq bot merged commit a4317a9 into carbon-design-system:main Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Fluid inputs: Label content overflow
7 participants