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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mherchel created an issue. See original summary.

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at OpenSense Labs commented
FileSize
input-disabled-3200644-2.patch1.25 KB
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 CreditAttribution: Gauravvvv at OpenSense Labs commented
Status: Active » Needs review
imalabya’s picture

imalabya
he/him
Primary language English
Location Bangalore
CreditAttribution: imalabya at Specbee for Drupal India Association 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
he/him
Primary language English
Location Bangalore
CreditAttribution: imalabya at Specbee for Drupal India Association commented
Status: Needs work » Needs review
FileSize
3200644-5.patch3.2 KB

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

vikashsoni’s picture

vikashsoni CreditAttribution: vikashsoni as a volunteer and at Zyxware Technologies commented
FileSize
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 CreditAttribution: cindytwilliams at Kanopi Studios commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
3200644-before.png30.52 KB
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 CreditAttribution: kleiton_rodrigues at CI&T commented
FileSize
site_before_3200644-8.png19.96 KB
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 CreditAttribution: vikashsoni as a volunteer and at Zyxware Technologies commented
FileSize
before_patch.png30.52 KB
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
CreditAttribution: mherchel 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 CreditAttribution: cindytwilliams at Kanopi Studios commented
Status: Needs work » Needs review
FileSize
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 CreditAttribution: cindytwilliams at Kanopi Studios commented
FileSize
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
CreditAttribution: mherchel commented
Status: Needs review » Reviewed & tested by the community
FileSize
interdiff-12-13.txt4.93 KB
3200644-13.patch5.85 KB
autocomplete-disabled.png57.02 KB
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
CreditAttribution: lauriii at Acquia 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
CreditAttribution: mherchel commented
Status: Needs work » Needs review
FileSize
3200644-15.patch5.85 KB
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 CreditAttribution: BhumikaVarshney as a volunteer and at OpenSense Labs commented
FileSize
before-patch.png30.43 KB
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 CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commented
FileSize
3200644-before_patch.png7.65 KB
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 CreditAttribution: djsagar at OpenSense Labs commented
Status: Needs review » Reviewed & tested by the community
FileSize
after-patch.png26.56 KB
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
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Fixed

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

Gauravvvv’s picture

Gauravvvv CreditAttribution: Gauravvvv at OpenSense Labs for DrupalFit commented

Status: Fixed » Closed (fixed)

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