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 height of label and input on NcInputField #4629

Closed
wants to merge 1 commit into from

Conversation

JuliaKirschenheuter
Copy link
Contributor

☑️ Resolves

Fix height for text spacing requirements. Checked with ARCToolkit.

🖼️ Screenshots

🏚️ Before 🏡 After
Screenshot from 2023-10-09 17-12-08 Screenshot from 2023-10-09 17-11-01

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

Signed-off-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
@ShGKme
Copy link
Contributor

ShGKme commented Oct 9, 2023

What is the difference between 🏚️ Before and 🏡 After? 🤨

@JuliaKirschenheuter
Copy link
Contributor Author

What is the difference between 🏚️ Before and 🏡 After? 🤨

truncated text

@susnux
Copy link
Contributor

susnux commented Oct 10, 2023

Why is the text clipped at the bottom? Also the input and the placeholder text is placed not where it should be (not related to the PR but seen on the screenshots. Something wrong is going on there).
And also the moved label is not centered, if you compare with that screenshot.

@susnux
Copy link
Contributor

susnux commented Oct 10, 2023

The increased height now causes this visual bug where the background overflows the bottom border for a short moment:

vokoscreenNG-2023-10-10_11-27-17.mp4

@raimund-schluessler
Copy link
Contributor

@JuliaKirschenheuter Your branch is quite a bit behind master. You have to please update it and resolve the conflicts, otherwise a discussion is difficult, because the values you want to adjust have changed in master.

But nonetheless, I can confirm the problem. It also occurs in the styleguide. It happens because this rule


is overwritte by this rule

@JuliaKirschenheuter JuliaKirschenheuter added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 10, 2023
@JuliaKirschenheuter
Copy link
Contributor Author

Thank you @raimund-schluessler and @susnux!

@susnux would you have some suggestions how to fix initial problem?

@JuliaKirschenheuter
Copy link
Contributor Author

@susnux do we have any feedback from designers? How could we move forwards?

@jancborchardt
Copy link
Contributor

@JuliaKirschenheuter @susnux I am not sure where design input is needed here? The bug seems to be merely CSS where the text is somehow set to overflow: none (or something similar). There should be no heightening of the input, but only fixing of the cutoff and/or white container size.

@susnux
Copy link
Contributor

susnux commented Oct 18, 2023

@jancborchardt it is about #4582

As that solution would fix this here without any adjustments. Meaning a design team decision on that issue (as requested by Marco)

@susnux susnux added the accessibility Making sure we design for the widest range of people possible, including those who have disabilities label Oct 19, 2023
@JuliaKirschenheuter
Copy link
Contributor Author

issue was solved here: #4718

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress accessibility Making sure we design for the widest range of people possible, including those who have disabilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants