Problem/Motivation

- This particular issue came out from the design QA review process conducted by @jwitkowski79.
- The line length for the labels below the input fields is pretty long compared to the font size. Can we make the width of the shorter? See attached screenshot for suggestions. Also, when there is a label, can we increase the margin to the field below just a tad? Included example in the attached screenshot below.

Steps to reproduce

- Visit the "Create a new account" page.

Proposed resolution

- See attached below for the proposed resolution.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#17 Desktop-before-patch.png74.09 KBdjsagar
#17 desktop-After-patch.png68.4 KBdjsagar
#17 mobile-screen-after-patch.png66.52 KBdjsagar
#17 Mobile-before-patch.png62.48 KBdjsagar
#16 3180281-16.patch1.45 KBanmolgoyal74
#15 3180281-15.patch1.41 KBPooja Ganjage
#12 3180281-after_patch-11-desktop.png56.29 KBabhijith s
#12 3180281-after_patch-11-mobile.png47.13 KBabhijith s
#11 3180281-11.patch1.41 KBkostyashupenko
#11 interdiff_10-11.txt531 byteskostyashupenko
#10 interdiff_7-10.txt789 byteskomalk
#10 3180281-10.patch859 byteskomalk
#10 After.png148.97 KBkomalk
#8 Screenshot from 2020-11-06 11-44-59.png51.18 KBpaulocs
#7 Before-patch.png122.27 KBkomalk
#7 After-patch.png111.47 KBkomalk
#7 interdiff_4-7.txt1.3 KBkomalk
#7 3180281-7.patch857 byteskomalk
#4 After-patch.png71.64 KBkomalk
#4 before-patch.png70.33 KBkomalk
#4 3180281-4.patch1009 byteskomalk
QBM6QyDQ.png118.7 KBproeung
0JjBPs0A.png216.26 KBproeung

Comments

proeung created an issue. See original summary.

proeung’s picture

proeung
she/her
commented
Title: [Olivero Theme] [Design QA]: Tighten the column count of the form helper text » [Olivero Theme] [Design QA]: Adjust the column count of the form helper text container
komalk’s picture

komalk commented
Assigned: Unassigned » komalk
komalk’s picture

komalk commented
Assigned: komalk » Unassigned
Status: Active » Needs review
StatusFileSize
new 3180281-4.patch1009 bytes
new before-patch.png70.33 KB
new After-patch.png71.64 KB

Attached the screenshot for references.
Review the patch.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

This patch sets the width of the text inputs and textareas to 100%. We need to limit the width of the description text.

komalk’s picture

komalk commented
Assigned: Unassigned » komalk
komalk’s picture

komalk commented
Assigned: komalk » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3180281-7.patch857 bytes
new interdiff_4-7.txt1.3 KB
new After-patch.png111.47 KB
new Before-patch.png122.27 KB

Worked on #5.
Attached screenshot for reference.
Review the patch.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Screenshot from 2020-11-06 11-44-59.png51.18 KB

Looks good to me.
Lets see if the maintainers agree to use 70% for the form item description. Or if we have to use JS to do it.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/themes/olivero/css/components/form.css
@@ -131,6 +131,7 @@ tr .form-item,
+  max-width: 70%;

This will also only be 70% on small screen widths, which is not desirable. Let's set the max-width to something like 70ch

komalk’s picture

komalk commented
Status: Needs work » Needs review
StatusFileSize
new After.png148.97 KB
new 3180281-10.patch859 bytes
new interdiff_7-10.txt789 bytes

Worked on #9.
Set the max-width to 60ch.
with using 70ch not getting the expected output.
Attached screenshot for reference.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
StatusFileSize
new interdiff_10-11.txt531 bytes
new 3180281-11.patch1.41 KB

Never used "ch" unit, but looks like it cleanly resolve this issue. can-i-use says we can use this unit and compatibility is completely matched to browser-requirements, except of Opera Mini only
Moreover, i have tested this patch manually in Firefox, Safari, Chrome, MS Edge, IE11 and it works as expected
I got one problem on yarn lint:css that "ch" unit is not presented in our stylelintrc rules, so i have added it.

Thoughts?

abhijith s’s picture

abhijith s commented
StatusFileSize
new 3180281-after_patch-11-mobile.png47.13 KB
new 3180281-after_patch-11-desktop.png56.29 KB

Applied patch #11 and it works fine.The labels below input fields are shortened.As mentioned in the #11 it has a wide compatibility over browsers.

Including screenshots after applying the patch:
Desktop
desktop

mobile
mobile

I'm moving it to RTBC

abhijith s’s picture

abhijith s commented
Status: Needs review » Reviewed & tested by the community
spokje’s picture

spokje
Primary language Dutch
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch #11 doesn't apply any more and needs a reroll.

Pooja Ganjage’s picture

Pooja Ganjage commented
StatusFileSize
new 3180281-15.patch1.41 KB

Hi,

Creating a patch for reroll.

Please review the patch.

Thanks.

anmolgoyal74’s picture

anmolgoyal74 commented
Status: Needs work » Needs review
StatusFileSize
new 3180281-16.patch1.45 KB
djsagar’s picture

djsagar commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Mobile-before-patch.png62.48 KB
new mobile-screen-after-patch.png66.52 KB
new desktop-After-patch.png68.4 KB
new Desktop-before-patch.png74.09 KB

Hi,

Patch #16 is applied successfully and tested.

working fine for me.

Thanks!

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

RTBC + 1

  • lauriii committed 33c3de6 on 9.2.x 
    Issue #3180281 by komalk, kostyashupenko, anmolgoyal74, Pooja Ganjage,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Issue tags: -Needs reroll

Committed 33c3de6 and pushed to 9.2.x. Thanks!

Leaving open for 9.1.x commit because I'm not sure we're allowed to make the change for the stylelint rules there.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Version: 9.1.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

No need to port to 9.1. Most people won't notice this :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.