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

RFC: Visible internal label for NcInputFields within the element #4126

Closed
susnux opened this issue May 16, 2023 · 16 comments
Closed

RFC: Visible internal label for NcInputFields within the element #4126

susnux opened this issue May 16, 2023 · 16 comments
Labels
3. to review Waiting for reviews design Design, UX, interface and interaction design
Milestone

Comments

@susnux
Copy link
Contributor

susnux commented May 16, 2023

I pretty sure this is nothing new, probably someone already uses something like this, but I wanted to know your opinions on this idea:

Having a visual label is most often required, but also often the placeholder is misused for it.
As placeholders should specify what input is correct or expected but not necessarily what field it currently is.

I also noticed that our NcSelect does now has a height of 48px vs all other input elements (36px), sometimes this looks odd together so I resize inputs to match the 48px. Which gives 12px extra space.

Combining this two things I came up with the idea to have the label be inside the element and only shrink and move when entering text.

So this is how it could look like (just the "Old", "Good new" and "Disabled" ignore the other ones):
mockup

@susnux susnux added question Further information is requested design Design, UX, interface and interaction design 0. to triage labels May 16, 2023
@susnux susnux added this to the 8.0.0 milestone May 31, 2023
@szaimen
Copy link
Contributor

szaimen commented Jun 16, 2023

I would honestly prefer a solution like in material design, like this:
image
So that it moved into the border itself.

@susnux
Copy link
Contributor Author

susnux commented Jun 16, 2023

I would honestly prefer a solution like in material design

This looks good too! What might be problematic is the height of the element.

For my suggestions we can use the standard height of 44px like buttons or the NcSelect component, so there are 40px content height which allow 15px main text + upper 13px label text + padding.

ℹ️ Edit:
Never mind, I tested that and with the text inside the border we also would just have a height of 44px (with 6px text on top of the border)

image

@szaimen
Copy link
Contributor

szaimen commented Jun 16, 2023

Also cc @nextcloud/designers

@marcoambrosini
Copy link
Contributor

@szaimen the advantage of the original proposal is that it doesn't interfere with the keyboard navigation though.
That's also the way airbnb does it. I'm a bit torn here

Screen.Recording.2023-06-23.at.13.56.36.mov

@szaimen
Copy link
Contributor

szaimen commented Jun 24, 2023

I found this video from Material design:

l13ubrf0-Label.text_3P.mp4

from here: https://m3.material.io/components/text-fields/guidelines

So both versions are fine by me :)

@nimishavijay
Copy link

Design wise both look good to me as well, I would assume that animating the break in the border may be slightly tricky to get right. So I would vote for the original proposal with one change: reduce the padding of the field when it is focused. Right now there is just a tad bit more space to the left than in the top or bottom. In the screenshot, the text should start from the blue line, currently it starts from the red line.

image

Another point to consider here is how this would work with icons, as many times the text field has a label with an icon or a placeholder text with an icon, so both of those should reduce size evenly.

@marcoambrosini
Copy link
Contributor

I've been testing this a bit and I have to take back what I said before about the focus outlines: it's impossible for this element to be focus and active at the same time.
@nimishavijay given that and what you raise about the icons it's probably better to have the label on the border so that there's more space for them? I don't think it should be difficult to get right.

@susnux
Copy link
Contributor Author

susnux commented Jun 28, 2023

To allow testing this kind of component I have created a PR for it: #4287
I also tried to cover all suggestions from this thread.

@nimishavijay
Copy link

nimishavijay commented Jun 28, 2023

This looks good! Some small alignment adjustments, with before/after screenshots:

  • In unfocused state:
    • for fields with icons: increase padding slightly on the left of the text. in the screenshot it is padding-inline-start: 34px;

image

  • Animation and focused state
    • the duration of the animation can be reduced to var(--animation-slow) which is 100ms (IMO 200ms would work better, but that's not a variable yet, so we can go for the snappier animation. what do you think? also @marcoambrosini)
    • also animate the color
    • what do you think about reducing the font size of the label to 12px? do we use 12px anywhere else? @marcoambrosini
    • there can be a little less spacing on the top of the label, in the screenshot it's inset-block-start: -4px
    • there can also be less spacing to the left of the label and placeholder. In the screenshot I've tried
      • for the field without icon: padding-inline: 6px for the placeholder and padding-inline: 8px 0 for the label
      • for the field with the icon: padding-inline-start: 32px for the placeholder and padding-inline: 34px for the label

The end results with those changes look like this. What do you think?

image
image

Besides that, it looks good! Also, I am concerned about accessibility, specifically the hover and focus states when the field is filled. Maybe @Pytal or @JuliaKirschenheuter can take a look to see if everything is ok? :)

@susnux
Copy link
Contributor Author

susnux commented Jun 29, 2023

the duration of the animation can be reduced to var(--animation-slow) which is 100ms (IMO 200ms would work better, but that's not a variable yet, so we can go for the snappier animation. what do you think?

I agree 200ms look better but yes we can go with animation-quick it is a lot snappier.

I implemented most of your suggestions, looks better :)

there can be a little less spacing on the top of the label, in the screenshot it's inset-block-start: -4px

For me this only works when setting the font size to 12px. Which I do not know is that good, looks a bit small?

@nimishavijay
Copy link

I experimented again, and I am ok with using 13px, especially since 12px is not used much elsewhere AFAIK. Updated comment :)

However, inset-block-start: -4px does look good with 13px label as well, otherwise it seems like there is not enough space above the label. Screenshot attached, top text field has -6px and bottom text field has -4px. What do you think?

image

@marcoambrosini
Copy link
Contributor

Given this, I would also suggest to use the version with the label on the border so everything can be spaced out properly and we don't have problems with leading icons.

@susnux
Copy link
Contributor Author

susnux commented Jul 1, 2023

@nimishavijay

However, inset-block-start: -4px does look good with 13px label as well, otherwise it seems like there is not enough space above the label.

I agree, with that change it looks less crowded 😃
I updated the preview PR with this change.


@marcoambrosini

I've been testing this a bit and I have to take back what I said before about the focus outlines: it's impossible for this element to be focus and active at the same time.

Maybe I do not understand the problem here, but I do not think you can have a focus-visible state different besides the active state, because as soon as you focus the element it will get active. So the active state is always the same as the focus-visible state.

given that and what you raise about the icons it's probably better to have the label on the border so that there's more space for them?

I do not think you should ever put an icon inside the input label, but instead set it as the leading icon of the component.
This way you do not have any issues like this (and it is always good to "read" as its size is not changed).

But even if you really need the icon within the label that is possible too without big adjustments (just add an icon slot):
image

@marcoambrosini
Copy link
Contributor

@susnux I mean a leading icon such as in search

Screenshot 2023-07-03 at 16 21 02

@susnux
Copy link
Contributor Author

susnux commented Aug 25, 2023

#4394

@susnux susnux closed this as completed Aug 25, 2023
@susnux susnux added 3. to review Waiting for reviews and removed question Further information is requested 0. to triage labels Aug 25, 2023
@szaimen
Copy link
Contributor

szaimen commented Aug 25, 2023

🎉🎉🎉🎉🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UX, interface and interaction design
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants