Spawned from #3194350: Implement new form element designs for Olivero

Olivero's autocomplete input does not have disabled styling

CommentFileSizeAuthor
#18 before-patch.png23.3 KBdjsagar
#18 after-patch.png26.56 KBdjsagar
#17 3200644-after_patch.png7.75 KBabhijith s
#17 3200644-before_patch.png7.65 KBabhijith s
#16 after-patch.png31.44 KBbhumikavarshney
#16 before-patch.png30.43 KBbhumikavarshney
#15 interdiff-13-15.txt2.68 KBmherchel
#15 3200644-15.patch5.85 KBmherchel
#13 autocomplete-disabled-rtl.png54.34 KBmherchel
#13 autocomplete-disabled.png57.02 KBmherchel
#13 3200644-13.patch5.85 KBmherchel
#13 interdiff-12-13.txt4.93 KBmherchel
#12 3200644-12.patch7.09 KBcindytwilliams
#11 3200644-11.patch4.93 KBcindytwilliams
#9 after_patch.png30.51 KBvikashsoni
#9 before_patch.png30.52 KBvikashsoni
#8 site_after_3200644-8.png20.12 KBkleiton_rodrigues
#8 site_before_3200644-8.png19.96 KBkleiton_rodrigues
#7 3200644-after.png30.51 KBcindytwilliams
#7 3200644-before.png30.52 KBcindytwilliams
#6 Screenshot from 2021-03-11 18-09-45.png252.26 KBvikashsoni
#5 3200644-5.patch3.2 KBimalabya
#2 Screenshot 2021-03-09 at 13.28.13.png14.25 KBgauravvvv
#2 input-disabled-3200644-2.patch1.25 KBgauravvvv
olivero-autocomplete-disabled.png213.07 KBmherchel

Comments

mherchel created an issue. See original summary.

gauravvvv’s picture

gauravvvv commented
StatusFileSize
new input-disabled-3200644-2.patch1.25 KB
new Screenshot 2021-03-09 at 13.28.13.png14.25 KB

I have attached a patch and after patch screenshot for reference. Please review.

gauravvvv’s picture

gauravvvv commented
Status: Active » Needs review
imalabya’s picture

imalabya
Primary language English
Location Bangalore
commented
Status: Needs review » Needs work

@Gauravmahlawat The issue is that the autocomplete fields don't have a style when they are disabled. Moving it to NW

imalabya’s picture

imalabya
Primary language English
Location Bangalore
commented
Status: Needs work » Needs review
StatusFileSize
new 3200644-5.patch3.2 KB

Added a patch that applied the disabled styling for autocomplete field.

vikashsoni’s picture

vikashsoni commented
StatusFileSize
new Screenshot from 2021-03-11 18-09-45.png252.26 KB

@imalabya i am getting error when applying the patch if i am missing some tell me check screenshot

cindytwilliams’s picture

cindytwilliams commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 3200644-before.png30.52 KB
new 3200644-after.png30.51 KB

Patch #5 applied cleanly via Composer, and the disabled autocomplete input field now has disabled styling. Marking RTBC.

Before patch:

After patch:

kleiton_rodrigues’s picture

kleiton_rodrigues commented
StatusFileSize
new site_before_3200644-8.png19.96 KB
new site_after_3200644-8.png20.12 KB

I applied patch and the input field now has disabled styling css.
It's good for me.

RTBC +1.

vikashsoni’s picture

vikashsoni commented
StatusFileSize
new before_patch.png30.52 KB
new after_patch.png30.51 KB

Applied patch working fine sharing the screenshot...

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Needs work

This looks great for normal left-to-right languages (such as english), but the styling does not work for RTL languages (such as arabic). These styles are controlled in the same file around line 32. We basically need to do the same thing there. We also should be adding the /* LTR */ comment after each CSS line that is LTR specific like the background-position property's line (see https://www.drupal.org/docs/develop/standards/css/css-formatting-guideli...).

cindytwilliams’s picture

cindytwilliams commented
Status: Needs work » Needs review
StatusFileSize
new 3200644-11.patch4.93 KB

Here is a patch that applies the disabled styling for autocomplete fields for both RTL and LTR languages, with /* LTR */ comments added where needed.

cindytwilliams’s picture

cindytwilliams commented
StatusFileSize
new 3200644-12.patch7.09 KB

I forgot to add the disabled color for RTL. Added that to this patch, along with the disabled styling for autocomplete fields for both RTL and LTR languages, and the /* LTR */ comments where needed.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new interdiff-12-13.txt4.93 KB
new 3200644-13.patch5.85 KB
new autocomplete-disabled.png57.02 KB
new autocomplete-disabled-rtl.png54.34 KB

The autocomplete-loading.module.pcss.css looks perfect. The reason why the commands failed in the patch above is because the compiled CSS in the patch didn't match the compiled CSS that core output when running yarn build:css. You'll want to make sure your dependencies are up to date by running yarn install from the /core directory.

I'm including an updated patch an interdiff here. Setting to RTBC since the only thing we're changing is the generated files.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/themes/olivero/css/components/autocomplete-loading.module.pcss.css
@@ -15,7 +15,14 @@
+  background-color: transparent;

I think it would be better to set this to --color--white like we do on form-text.pcss.css to ensure the text has sufficient contrast

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Needs review
StatusFileSize
new 3200644-15.patch5.85 KB
new interdiff-13-15.txt2.68 KB

I think it would be better to set this to --color--white like we do on form-text.pcss.css to ensure the text has sufficient contrast

Done!

bhumikavarshney’s picture

bhumikavarshney commented
StatusFileSize
new before-patch.png30.43 KB
new after-patch.png31.44 KB

Hi @mherchel,
The patch applies cleanly and also now the color is set to --color--white.
Thanks.

abhijith s’s picture

abhijith s commented
StatusFileSize
new 3200644-before_patch.png7.65 KB
new 3200644-after_patch.png7.75 KB

Applied patch #15 and it works fine.Adding screenshots.

Before patch:
before

After patch:
after

RTBC +1

djsagar’s picture

djsagar commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new after-patch.png26.56 KB
new before-patch.png23.3 KB

Applied patch #15 and working as accepted so moving this as on RTBC.

Adding attachment.

  • lauriii committed 88d8300 on 9.2.x 
    Issue #3200644 by mherchel, cindytwilliams, Gauravmahlawat, imalabya,...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Committed 88d8300 and pushed to 9.2.x. Thanks!

gauravvvv’s picture

gauravvvv commented

Status: Fixed » Closed (fixed)

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