Problem/Motivation

Tour module popup close button size is too small. In Umami, the size of close button is larger and seems like a more reasonable size.

Olivero:

Umami:

Steps to reproduce

  1. Install Drupal with Umami Demo profile.
  2. Switch default theme to Olivero.
  3. Go to home page.
  4. Click on tour link in upper right.
  5. Result: The close button is quite small.
  6. Expected: Close button would be bigger and more similar to the Umami Demo theme size.

Proposed resolution

Increase the size to be more similar to Umami.

Remaining tasks

  1. Create patch
  2. Review patch
  3. Test patch
  4. Commit

User interface changes

Close button will be larger.

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 Screenshot 2022-02-23 at 5.16.49 PM.png1 MBSatyajit1990
#10 After patch.png110.76 KBSatyajit1990
#10 Before patch.png114.14 KBSatyajit1990
#6 3256003-6.patch796 bytesranjith_kumar_k_u
#5 3256003-5.patch392 bytesandregp
#4 diff_3256003_3-4.txt310 bytesandregp
#4 3256003-4.patch799 bytesandregp
#3 3256003-3.patch753 bytesandregp
Screenshot 2021-12-25 at 9.20.19 PM.png118.71 KBgauravvvv
Screenshot 2021-12-25 at 9.19.35 PM.png57.82 KBgauravvvv
Show 5 more files

Comments

Gauravmahlawat created an issue. See original summary.

andregp’s picture

andregp CreditAttribution: andregp at CI&T commented
Assigned: Unassigned » andregp

I'll work on this issue

andregp’s picture

andregp CreditAttribution: andregp at CI&T commented
Assigned: andregp » Unassigned
Status: Active » Needs review
StatusFileSize
new 3256003-3.patch753 bytes

Here's a simple patch to solve this issue.

andregp’s picture

andregp CreditAttribution: andregp at CI&T commented
StatusFileSize
new 3256003-4.patch799 bytes
new diff_3256003_3-4.txt310 bytes

Fixing coding standards.

andregp’s picture

andregp CreditAttribution: andregp at CI&T commented
StatusFileSize
new 3256003-5.patch392 bytes

I keep getting the error No files matching the pattern "/var/www/html/core/themes/olivero/css/components/tour.css" were found. on the tests above (#3 and #4). So instead of creating a new file for the custom style, I just added it to a file that already exists.

(I'm not providing an interdiff as it's a completely different approach.)

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commented
StatusFileSize
new 3256003-6.patch796 bytes
andregp’s picture

andregp CreditAttribution: andregp at CI&T commented

@ranjith_kumar_k_u the olivero.libraries.yml only loads the button.css file not the button.pcss.css so why did you change it too on your patch? What does you patch do differently from the previous one?
When providing a new patch It's a good practice to explain the reason so the community can know what you're trying to fix.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: kristen pol at Itty Bitty Byte for Salsa Digital commented
Issue tags: +Bug Smash Initiative, +Needs manual testing

@andregp I thought that the pss.css files were used to generate the .css files which is why the:

/*
 * DO NOT EDIT THIS FILE.
 * See the following change record for more information,
 * https://www.drupal.org/node/3084859
 * @preserve
 */

is at the top of the .css files. No? But, I haven't done much frontend dev in a long time and have only worked with Sass/Less before.

Tagging for testing since tests are passing.

Patch applies cleaning to Drupal 9.3 and 9.4 and with offsets for Drupal 10:

[drupal-10.0.x-dev/10.0.x] [drupal-10.0.x-dev]$ patch -p1 < 3256003-6.patch 
patching file core/themes/olivero/css/components/button.css
Hunk #1 succeeded at 202 (offset 13 lines).
patching file core/themes/olivero/css/components/button.pcss.css
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: kristen pol at Itty Bitty Byte for Salsa Digital commented
Issue summary: View changes
Issue tags: +Needs issue summary update

Added testing notes. Also tagging for the issue summary to be cleaned up such as using the issue summary template.

Satyajit1990’s picture

Satyajit1990 CreditAttribution: Satyajit1990 at Material for Salsa Digital commented
Issue summary: View changes
StatusFileSize
new Before patch.png114.14 KB
new After patch.png110.76 KB

Hi @Kristen Pol,

Verified with Patch #6
Version - 9.4
Theme - Olivero
Testing result : Now Close button is bigger and more similar to the Umami Demo theme size.

Note : Patch file is not working on 9.3

Satyajit1990’s picture

Satyajit1990 CreditAttribution: Satyajit1990 at Material for Salsa Digital commented
StatusFileSize
new Screenshot 2022-02-23 at 5.16.49 PM.png1 MB
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: kristen pol at Itty Bitty Byte for Salsa Digital commented
Issue tags: -Needs manual testing

Thanks for testing @Satyajit1990. You don't have to include files of simplytest.me or DrupalPod not working.

But, I have found out from @catch that when the patch applies to more than one version, we can just test on 9.4 or 10, i.e. we don't have to test on all versions... just ones with different MRs/patches.

Assuming this CSS approach is okay, all that's needed is the issue summary update.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: kristen pol at Itty Bitty Byte for Salsa Digital commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update

Updated the issue summary to use the template. Marking RTBC based on the above.

  • lauriii committed c606891 on 10.0.x 
    Issue #3256003 by andregp, ranjith_kumar_k_u, Satyajit1990,...

  • lauriii committed 3e4ed27 on 9.4.x 
    Issue #3256003 by andregp, ranjith_kumar_k_u, Satyajit1990,...
lauriii’s picture

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

Committed c606891 and pushed to 10.0.x. Also cherry-picked to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

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