[RFC]: NcInputField - visible inner label inside input about nextcloud-vue HOT 39 CLOSED

We just talked about this in the design meeting and we will go with the last version by @susnux (48px) :)

from nextcloud-vue.

Let's wait for another to read this thread and share their thoughts.

Sure :)

For testing you can find the old proposal with the inner label in branch feat/nciputfield-v2

[...] while using considerably less space.

BTW the reason why I prefer unify the height of input elements are forms where you have multiple components, they look odd if the height does not match:

(Of course the inner label is missing on the select, but even if it would also support it, the height would not match.)

from nextcloud-vue.

If we make NcSelect smaller, we do not gain anything. It still looks weird then because the outline might match but the label on the border will not.

I think the simplest solution would be the label within and make input fields the same height as NcSelect.
This way we solve multiple problem (background colors, height issues, text height issues, clickable area for AAA) and it will look better even with a label next to a NcSelect.

(I think the problem from design perspective (at least for me) is that the label on the border is a new design element and will always look odd compared to other components, either they need this option too or no border label is used when next to an other element. The inner label does not have this problem because the border is "not broken" so if both are the same height they still match visually)

from nextcloud-vue.

For example, what this place in apps' navigation should look like?

This part is already custom styled, the input was sized to 44px instead of 36px and made much rounder

from nextcloud-vue.

Ok, spacing-wise, @marcoambrosini's latest proposal looks much better indeed. It's not as big and jarring. Let's go for this.

For the truncation issue, we need to make sure the label container is big enough and/or the text label is is centered. As @marcoambrosini said this can/must be solved independently of the overall solution.

@marcoambrosini can you please stay on this to help with the CSS details?

from nextcloud-vue.

I personally like both versions.

But I think the first approach is less error prone, as you do not have to think about any problems with the background where you use the components.
Moreover the outline / border-box is more clear as what you see is the outline and the label is only moved within the borders.

But as with the initial discussion, this is a design decision so cc @jancborchardt @nimishavijay @marcoambrosini

from nextcloud-vue.

the first approach is less error prone, as you do not have to think about any problems with the background where you use the components.

Agree with this assessment @susnux, and thanks @ShGKme for bringing this up. I would agree with switching to the first proposal as it will look much better in all sorts of environments.

from nextcloud-vue.

Back then I raised concerns about the readability of the text. The label is almost tangent to the border and might break line height accessibility requirements. And padding around input text is very little. I think that the readability of both input text and label text is more important than this background edge case.

@jancborchardt You can get a sense for it in the action menu here #4287 (comment)

from nextcloud-vue.

@ShGKme @JuliaKirschenheuter do we know how the proposed solution would fare accessibility-wise?

from nextcloud-vue.

@ShGKme @JuliaKirschenheuter do we know how the proposed solution would fare accessibility-wise?

It would be even better because currently clickable area of inputs is smaller than the required 44px.

Apart from that, there is no difference, it's only about input height and label position change for about 4px.

from nextcloud-vue.

The clickable area of elements doesn't have to be 44px to meet the aria AA target size success criterion. The line height of the text needs to be at least 1.5 though and putting a border at the very end of the line height looks wrong to me anyway.

This is how proper spacing around text looks in a contained label text input:

Airbnb

Material

Both are over 50px to provide the necessary whitespace around the text. By contrast, this is our component:

I leaned towards the other version because whilst using less space, it provides better readability of the text.

from nextcloud-vue.

The clickable area of elements doesn't have to be 44px to meet the aria AA target size success criterion.

But for AAA. But also important: Unify element height.

The line height of the text needs to be at least 1.5 though

No it is not necessary, WCAG only defines this gor text blocks to help reading it. For this element we just need to ensure the padding to the bottom.

So 13px label + 7px padding + 15px text = 35px height.
So there are still 4px for borders (= 39px) and 5px additional padding for label top and bottom.

But as I said we should unify the height. NcSelect is even more height.

from nextcloud-vue.

But for AAA

We are aiming at AA as far as I understand. The button component is an exception.

There is no need to unify the height of buttons and inputs. It might be convenient, yes, but it's by no means a priority. So I don't agree that we should unify the height.
Text readability is far more important. No major UI library seems to be bothered with equalizing button height with input height.

If we really want to move to the inner label, I think we should go above 50px height like in the previous examples.

from nextcloud-vue.

We can unify with NcSelect, then it even works for a11y (screenshot was taken with 1.5 arctoolkit so it looks a bit off but it shows that is succeeds the tests):

This has a height of 48px like NcSelect. This way it looks consistent when using input + select.
And even fit in the table rows (files etc) which have a height of 50px.

from nextcloud-vue.

It looks better now, but I still think the other option looks better while using considerably less space.
Let's wait for another @jancborchardt @szaimen @nimishavijay to read this thread and share their thoughts.

from nextcloud-vue.

Dear @jancborchardt / @marcoambrosini, @nimishavijay,

i would vote for unifying the height of input elements. Would you have any points against that?

Dear design team, we have to move forwards on our a11y issues and need a decision asap. Please give us a feedback how could we move forwards 🙏. Thanks a lot!

from nextcloud-vue.

Dear @jancborchardt / @marcoambrosini, @nimishavijay,

i would vote for unifying the height of input elements. Would you have any points against that?

Dear design team, we have to move forwards on our a11y issues and need a decision asap. Please give us a feedback how could we move forwards 🙏. Thanks a lot!

I still think that having thr label in the border looks better but of coure having the same height for select and input field also makes sense. I guess we cannot reduce the height of the select component to match the input field? If so I am fine with this change 👍

from nextcloud-vue.

I still think that having thr label in the border looks better

In this case, how should be work with different backgrounds? For example, on the login page (not the only one example).

from nextcloud-vue.

In this case, how should be work with different backgrounds? For example, on the login page (not the only one example).

There a input field with label outside should be used IIRC. cc @nextcloud-libraries/designers on this

from nextcloud-vue.

By the way, does the current solution fit AAA requirements about text readability, text height/spacing?

from nextcloud-vue.

By the way, does the current solution fit AAA requirements about text readability, text height/spacing?

I'm not an expert, but that seems (okay). Could be better, but it is readable:

from nextcloud-vue.

But there is some other problem: if label is not visible (case where input field is empty):

from nextcloud-vue.

By the way, does the current solution fit AAA requirements about text readability, text height/spacing?

I'm not an expert, but that seems (okay). Could be better, but it is readable:

By the current I meant - in the @nextcloud/vue's master

from nextcloud-vue.

By the current I meant - in the @nextcloud/vue's master

No, because of this problem: nextcloud/server#36977 (comment)

from nextcloud-vue.

@marcoambrosini, @jancborchardt, @nimishavijay

would you approve a current state?

from nextcloud-vue.

We just talked about this in the design meeting and we will go with the last version by @susnux (48px) :)

@marcoambrosini Do you think we should make an optional behavior to return the previous or "small" layout?

Or it fits fine most of the usages? I'm a bit afraid of layouts that might have counted on this size.

For example, what this place in apps' navigation should look like?

from nextcloud-vue.

Do you think we should make an optional behavior to return the previous or "small" layout?

@ShGKme I think that by doing this we're introducing more problems that we're solving, we're suddenly increasing the height of this element by 12 pixels. @jancborchardt I leave this in your hands for further decisions on this as I just don't see the point of this change.
There's no accessibility issue whatsoever with the current input, just saying this for the record because it seemed yesterday that @jancborchardt and @JuliaKirschenheuter thought there were.
The design team couldn't explain yesterday why the select component is suddenly so tall, so "matching the select height" becomes even less of a reason for doing this.

This part is already custom styled, the input was sized to 44px instead of 36px and made much rounder

The height of the search conversation input is 36px, custom style is the pill border

Would you approve a current state?

@JuliaKirschenheuter the label needs to be vertically centered in the input when the input is not active

from nextcloud-vue.

...actually, here's one last proposal keeping the current style but bringing it up to 44px (input size not counting label) and bringing the select height back down to 44px too.
As said yesterday, forms should in general be on "main" backgrounds and the login is an edge case that can be handled separately. I included a "darker" background version which doesn't look bad at all.

from nextcloud-vue.

Hi @marcoambrosini,

Thank you for your anwer! But i disageree with you in that point:

There's no accessibility issue whatsoever with the current input, just saying this for the record because it seemed yesterday that @jancborchardt and @JuliaKirschenheuter thought there were.

Please look in the truncation problem here nextcloud/server#36977 (comment). And this is an accessibility problem. It must be checked whether changing text spacing leads to text truncation, overlap, or loss of functionality.

from nextcloud-vue.

...actually, here's one last proposal keeping the current style but bringing it up to 44px (input size not counting label) and bringing the select height back down to 44px too. As said yesterday, forms should in general be on "main" backgrounds and the login is an edge case that can be handled separately. I included a "darker" background version which doesn't look bad at all.

Could we please transfer it to another improvement issue?

from nextcloud-vue.

Please look in the truncation problem here nextcloud/server#36977 (comment)

That is a separate issue @JuliaKirschenheuter and has nothing to do on whether we go with contained label vs label on border

from nextcloud-vue.

That is a separate issue @JuliaKirschenheuter and has nothing to do on whether we go with contained label vs label on border

It has. Please compare truncation:

After	Before

from nextcloud-vue.

As said yesterday, forms should in general be on "main" backgrounds and the login is an edge case that can be handled separately.

That is not fully true, yes the login is - together with the dashboard widgets - an edge case. But we quite often have at least a darker background. E.g. the hover state (user list rows) etc.

So we are not really independent to the background

from nextcloud-vue.

It has. Please compare truncation:

There's some container clipping going on there and this issue is really not about that @JuliaKirschenheuter. That can happen with both designs and can be solved independently from the path we take.

E.g. the hover state

Text inputs should not be within elements that one can click on in the first place, so there should be no hover state background change surrounding them.

from nextcloud-vue.

Ok, which way should be finally go?

from nextcloud-vue.

I'm stepping back for good after that final proposal, Jan will weigh in with a final decision soon. The truncation of the component can be solved regardless of the outcome of the issue.

from nextcloud-vue.

@marcoambrosini can we close this issue as #4718 is merged?

from nextcloud-vue.

For testing you can find the old proposal with the inner label in branch feat/nciputfield-v2

i guess we don't need it as last decision was to follow this way #4582 (comment)

from nextcloud-vue.

@marcoambrosini can we close this issue as #4718 is merged?

Yes!

from nextcloud-vue.