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

Error message does not take up space in form #613

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

daviddkkim
Copy link
Contributor

Closes #232

Changes

// Describe what you changed
I added new styling container on top of the form error message so that when an error message appears, it does not take up spacing.

Screenshots

Screen.Recording.2021-04-25.at.3.03.53.PM.mov

There is one caveat. This means that developers using clockface have to add in the right spacing after the input element so that this does not happen:

Screen Shot 2021-04-25 at 3 33 44 PM

Or I can make it so that it has built in spacing out of the box. @mavarius do you have thoughts on which one might be preferable?

I see good reasons for having pre-defined spacing to prevent the overlap (easy out of the box) and also not having it (more freedom).

// Add screenshots here if relevant

Checklist

Check all that apply

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@daviddkkim daviddkkim changed the title Adding support for help text -> error text conversion Error message does not take up space in form Apr 25, 2021
@mavarius
Copy link
Collaborator

I think having the built-in spacing is good. It means all our forms will be a bit more vertically spaced, but that might be ok. Just review how it looks across the application to avoid any unforeseen side effects.

@daviddkkim
Copy link
Contributor Author

daviddkkim commented May 7, 2021

Example1Spacing
LabelSpacing

@mavarius So here's a snippet of what the forms will look like with "built-in" spacing. I'm starting to think that we should leave the error message spacing behavior alone because built-in spacing throws off the vertical rhythm of the form elements and without built-in spacing, the error message overlaps with the component below. Especially if you look at the label creation form where there are multiple inputs in the form, it looks off

But I am all for changing the behavior of when there is help text + error text!

What do you think about the spacing issue?

@mavarius
Copy link
Collaborator

We could reduce the distance between the bottom of the error text and the top of the next element. If we absolute position the error text, then we can maintain a consistent distance between elements and have just a little less space when the error text is present without overlapping text.

Imagine this, but with 4px between the bottom of the error text and the top of the next label.
image

@daviddkkim
Copy link
Contributor Author

daviddkkim commented May 15, 2021

Yeah I like this. More spacing less crowded.
Frame 1

1st pictures are the original, the second nad third are the new forms with the built in spacing everywhere.
I posted this in UI channel just to make sure others are okay with this change too !

@mavarius

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation errors should not shift input fields
2 participants