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

[Visual bugfix] Match the original mockups for PastanagaUI in regards of the error messages in form field elements #5115

Merged
merged 7 commits into from Aug 24, 2023

Conversation

sneridagh
Copy link
Member

@sneridagh sneridagh commented Aug 23, 2023

Bug

Edit: It seems that we overlooked the original error messages in PastanagaUI form fields elements for over six years (shame on us) without noticing (or caring).

We realised (about time), when trying to fix this overlap:

image

Original mockup

This is the original mockup from the original PastanagaUI mocks:

image

Fixed version

image

@netlify
Copy link

netlify bot commented Aug 23, 2023

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit 1848d6e
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/64e76834a3abc200089305f7

@cypress
Copy link

cypress bot commented Aug 23, 2023

Passing run #6988 ↗︎

0 553 20 0 Flakiness 0

Details:

Fix Input alignment
Project: Volto Commit: 1848d6edb0
Status: Passed Duration: 13:36 💡
Started: Aug 24, 2023 2:28 PM Ended: Aug 24, 2023 2:41 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@tisto
Copy link
Sponsor Member

tisto commented Aug 23, 2023

@sneridagh the gap LGTM. One observation that we got from our client was that they do not see that they can enter something into the input fields that are just red blocks. There is no way a user can tell that the red block is actually an input field. I am wondering if we could change this and use a red border instead. Though, I understand that this might be too intrusive and a change that we can only apply in Volto 17.

@stevepiercy
Copy link
Collaborator

It's a small improvement. I hope that there is a larger plan to overhaul the design of inputs to actually resemble plain old familiar HTML5 inputs instead of lines that don't look anything like an input.

@sneridagh
Copy link
Member Author

@tisto I did a bit of archeological investigation after your comment.

You won't believe what I've found, but it seems that we never finished implementing the initial PastanagaUI form style guide. In fact, the red background comes directly from SemanticUI (!!).

These are the original PastanagaUI form elements mocks:

image

So, we could discuss with the team and treat this as a "fix" since it never had the original intended style (although years have passed :P )

@sneridagh
Copy link
Member Author

@tisto @stevepiercy of course, there is always Quanta style form elements in the horizon...

@sneridagh
Copy link
Member Author

@tisto @stevepiercy I can make it look it like this, as originally intended:

image

@stevepiercy
Copy link
Collaborator

@tisto @stevepiercy of course, there is always Quanta style form elements in the horizon...

Oh, you tease! 😉 Slides 26 and 43 for "before" and "after": https://www.slideshare.net/sneridagh/meet-quanta-plones-style-guide

@stevepiercy
Copy link
Collaborator

@tisto @stevepiercy I can make it look it like this, as originally intended:

image

I much prefer this. Fewer monstrous red blocks without meaning, and it gives the hint of where to click. I think that would be good enough, assuming Quanta styles will arrive in Volto 17.

@sneridagh
Copy link
Member Author

@stevepiercy unfortunately, Quanta styles don't have a milestone yet...

@tisto
Copy link
Sponsor Member

tisto commented Aug 23, 2023

@sneridagh that would be awesome! Then we could just consider this to be a "visual bugfix" :)

@sneridagh sneridagh changed the title Fix overlap of the error message with the first field in case that the first field is one of the errored fields [Visual bugfix] Match the original mockups for PastanagaUI in regards of the error messages in form field elements Aug 23, 2023
@sneridagh
Copy link
Member Author

@plone/volto-team Hi team!

We realised of this situation this morning. It seems that for some reason, we didn't realised that we never fulfilled the original PastanagaUI mocks in regards of the error messages in form field elements.

Although this change, in a strict sense, would be considered a breaking change, I'd like to mark it as a "Visual Bugfix" and declare it as "non-breaking" in order to include it also in Volto 16.

What do you think? Any opinions against it? Please let me know asap via this PR or privately.
Thanks a lot!

@tiberiuichim
Copy link
Contributor

I'm +1 on merging it. It fixes bad UI.

.ui.form {
.ui.pointing.label:before {
content: initial;
}
Copy link
Sponsor Member

@davisagli davisagli Aug 23, 2023

Choose a reason for hiding this comment

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

@sneridagh I think we should remove pointing from https://github.com/plone/volto/blob/master/src/components/manage/Widgets/FormFieldWrapper.jsx#L99 instead of changing the styles for .pointing. Semantically, nothing is pointing at anything now.

We could also add a new class in FormFieldWrapper and apply our styles to that class. I think that would help avoid breaking changes because if someone has shadowed FormFieldWrapper.jsx, the appearance won't change.

Copy link
Member Author

Choose a reason for hiding this comment

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

@davisagli done, please take a look.

@ksuess
Copy link
Member

ksuess commented Aug 24, 2023

Very welcome enhancement.

Maybe set flex-grow on class wrapper to 0. That would align the border at the bottom of label and field.

grafik

@sneridagh
Copy link
Member Author

@ksuess thanks for the heads up! Pushed!

image

@sneridagh sneridagh merged commit 54d5119 into master Aug 24, 2023
53 checks passed
@sneridagh sneridagh deleted the fixoverlapfirstformfield branch August 24, 2023 15:37
sneridagh added a commit that referenced this pull request Aug 25, 2023
… of the error messages in form field elements (#5115)
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.

None yet

6 participants