Olivero: repat placeholder CSS for IE11.

Steps to reproduce:

1. go to olivero/css/components/form.pcss.css file.
2. we have written IE11 specific css for placeholder. :-ms-input-placeholder {color: var(--color--gray-30) !important; } which is not required. Because yarn compiles the ::placeholder to :-ms-input-placeholder for IE11 support.
3. Adding screen-recording to better understanding.

Resolution: 

::placeholder {
  color: var(--color--gray-30);
}

/* IE 10 and 11 needs this set as important. */
:-ms-input-placeholder {
  color: var(--color--gray-30) !important;
}

This code compiles to 

:-ms-input-placeholder {
  color: #7e96a7;
}

::placeholder {
  color: #7e96a7;
}

/* IE 10 and 11 needs this set as important. */

:-ms-input-placeholder {
  color: #7e96a7 !important;
}

So we can remove this extra line of code.

/* IE 10 and 11 needs this set as important. */
:-ms-input-placeholder {
  color: var(--color--gray-30) !important;
}
CommentFileSizeAuthor
#28 3212691-28.patch914 bytesimmaculatexavier
#23 remove placeholder CSS for IE11.patch898 bytessauravkashyap
#22 reroll_diff_8-22.txt2.57 KBpooja saraah
#22 3212691-22.patch1.86 KBpooja saraah
#11 After Patch 3212691.png1.46 MBchetanbharambe
#11 Before Patch 3212691.png1.36 MBchetanbharambe
#8 3212691.8.patch1.85 KBsakthivel m
#7 After-patch.PNG63.25 KBsakthivel m
#7 Before-patch.PNG66.7 KBsakthivel m
#6 3212691-6.patch898 bytesgauravvvv
Screen Recording 2021-05-07 at 10.59.28.mov6.66 MBgauravvvv

Comments

Gauravmahlawat created an issue. See original summary.

gauravvvv’s picture

gauravvvv commented
Issue summary: View changes
mohitw’s picture

mohitw commented
Project: Drupal core » Olivero
Version: 9.3.x-dev » 8.x-1.x-dev
Component: Olivero theme » Code
mohitw’s picture

mohitw commented
Project: Olivero » Drupal core
Version: 8.x-1.x-dev » 9.3.x-dev
Component: Code » ajax system
mohitw’s picture

mohitw commented
Component: ajax system » Olivero theme
gauravvvv’s picture

gauravvvv commented
Status: Active » Needs review
StatusFileSize
new 3212691-6.patch898 bytes

I have removed the extra line of code and verified it on IE as well. Please review.

sakthivel m’s picture

sakthivel m commented
Status: Needs review » Needs work
StatusFileSize
new Before-patch.PNG66.7 KB
new After-patch.PNG63.25 KB

#6 patch applied successfully. The patch is not working.

I have attached screenshot

sakthivel m’s picture

sakthivel m commented
Status: Needs work » Needs review
StatusFileSize
new 3212691.8.patch1.85 KB

#8 Please review the patch

gauravvvv’s picture

gauravvvv commented
Status: Needs review » Closed (works as designed)

My bad. Works as designed.

gauravvvv’s picture

gauravvvv commented
Status: Closed (works as designed) » Needs review
chetanbharambe’s picture

chetanbharambe commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Before Patch 3212691.png1.36 MB
new After Patch 3212691.png1.46 MB

Verified and tested patch #8.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: Home page
# Click on Search icon -> Press Enter key
# Go to search input field
# User is able to see repeated placeholder CSS on IE11 Browser

Expected Results:
# User should not see repeated placeholder CSS on IE11 Browser

Actual Results:
# Currently User is able to see repeated placeholder CSS on IE11 Browser

Can be a move to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 3212691.8.patch, failed testing. View results

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Reviewed & tested by the community

Unrelated failure.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/themes/olivero/css/components/form.css
@@ -10,20 +10,6 @@
-/* IE 10 and 11 needs this set as important. */
...
-  color: #7e96a7 !important;

Is there a specific reason we can remove the !important?

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Category: Bug report » Task
Priority: Normal » Minor
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Internet Explorer 11

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

gauravvvv’s picture

gauravvvv commented
Parent issue: » #3253156: [meta] Remove IE11 Support from Olivero
gauravvvv’s picture

gauravvvv commented

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: Olivero: repat placeholder CSS for IE11 » Olivero: remove placeholder CSS for IE11
Version: 9.5.x-dev » 10.0.x-dev
Status: Needs review » Needs work
Issue tags: +Needs reroll

We can definitely drop this from 10.0.x as we no longer support IE11.

pooja saraah’s picture

pooja saraah commented
StatusFileSize
new 3212691-22.patch1.86 KB
new reroll_diff_8-22.txt2.57 KB

Attached reroll patch against 10.0.x

sauravkashyap’s picture

sauravkashyap commented
StatusFileSize
new remove placeholder CSS for IE11.patch898 bytes

i have removed the extra line code

smustgrave’s picture

smustgrave commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Issue tags: -Needs reroll

Looks like #23 passes current 9.5 tests if anyone with IE can test.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

As per https://caniuse.com/css-placeholder :-ms-input-placeholder was only used until Edge 18 which is no longer supported (and IE10/11, which is not listed on caniuse for some reason)

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Doesn't apply against 10.x

immaculatexavier’s picture

immaculatexavier commented
Status: Needs work » Needs review
StatusFileSize
new 3212691-28.patch914 bytes

Rerolled patch against 10.0.x

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Issue tags: -Needs reroll

  • catch committed 8b8be72 on 10.1.x 
    Issue #3212691 by Sakthivel M, Gauravmahlawat, immaculatexavier,...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!

  • catch committed 7756024 on 10.0.x 
    Issue #3212691 by Sakthivel M, Gauravmahlawat, immaculatexavier,...

Status: Fixed » Closed (fixed)

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