Problem/Motivation

A pixel or two higher would look right.

Steps to reproduce

Check any accordion element for example - go to /admin/config/system/site-information

Proposed resolution

Add margin-top: -0.125rem; in .claro-details__summary::before

Remaining tasks

Review

User interface changes

Before/After

image

API changes

NA

Data model changes

NA

Release notes snippet

NA

CommentFileSizeAuthor
#11 Screenshot at Jan 19 12-42-09.png148.15 KBshweta__sharma
#11 Screenshot at Jan 19 12-41-17.png199.51 KBshweta__sharma
#7 after-applying-patch.png5.41 KBVidushi Mehta
#7 before-applying-patch.png5.55 KBVidushi Mehta
#6 Screenshot from 2023-08-16 18-01-20.png3.79 KBkostyashupenko
#6 3377198-6.patch1.15 KBkostyashupenko
#4 vertical-aligned-chevron.png5.29 KBVidushi Mehta
#3 3377198 after patch.png9.87 KBHarish1688
#3 3377198-3.patch1.17 KBHarish1688
Screen Shot 2023-07-26 at 1.03.13 PM.png10.76 KBbnjmnm

Issue fork drupal-3377198

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bnjmnm created an issue. See original summary.

Harish1688’s picture

Harish1688 CreditAttribution: Harish1688 at Material commented

looking for this issue..

Harish1688’s picture

Harish1688 CreditAttribution: Harish1688 at Material commented
Status: Active » Needs review
FileSize
3377198-3.patch1.17 KB
3377198 after patch.png9.87 KB

Hi,

Reproduced the issue on local and created a patch for issue 'details up chevron not quite vertically aligned in forced color mode'. attached a image for reference.

Test Steps:
1. setup the Drupal 11.x and claro theme as backend theme=.
2. moved on path 'admin/config/system/site-information' and active the forced color in browser, and verified the issue (chevron not align vertically).
3. create 3377198-3.patch , Need Review.

After Patch:
issue image

Vidushi Mehta’s picture

Vidushi Mehta CreditAttribution: Vidushi Mehta at gai Technologies Pvt Ltd commented
Status: Needs review » Reviewed & tested by the community
FileSize
vertical-aligned-chevron.png5.29 KB

Reviewed the patch, the chevron is vertically top aligned after patch.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/themes/claro/css/components/details.css
@@ -310,7 +310,7 @@ td .claro-details {
+    margin-top: calc(0.125rem / -1);

Why not just

margin-top: -0.125rem;

?

Side-by-side screenshots from before and after would help as well.

kostyashupenko’s picture

kostyashupenko
Primary language Russian
Location Omsk
CreditAttribution: kostyashupenko at Skilld commented
Status: Needs work » Needs review
FileSize
3377198-6.patch1.15 KB
Screenshot from 2023-08-16 18-01-20.png3.79 KB

details chevron

Vidushi Mehta’s picture

Vidushi Mehta CreditAttribution: Vidushi Mehta at gai Technologies Pvt Ltd commented
FileSize
before-applying-patch.png5.55 KB
after-applying-patch.png5.41 KB

I've reviewed the patch #6 but still find the alignment issue, Attaching the before and after screenshots for the same. I am keeping the status same as Needs review so that we'll have more reviews on this

kostyashupenko’s picture

kostyashupenko
Primary language Russian
Location Omsk
CreditAttribution: kostyashupenko at Skilld commented

We don't need top alignment. It should be vertically centered

smustgrave’s picture

smustgrave CreditAttribution: smustgrave commented
Status: Needs review » Reviewed & tested by the community

Seems suggestion in #5 worked.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 3377198-6.patch, failed testing. View results

shweta__sharma’s picture

shweta__sharma CreditAttribution: shweta__sharma at OpenSense Labs commented
FileSize
Screenshot at Jan 19 12-41-17.png199.51 KB
Screenshot at Jan 19 12-42-09.png148.15 KB

Looks like this is already aligned I measured with a ruler and the arrow top and bottom height is 19px
See screenshots for reference.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
CreditAttribution: bnjmnm at Acquia commented

@shweta__sharma Perhaps I'm misreading your images, but the issue specifies this is related the up-pointing chevron, and both screenshots you provided are examining the down-pointing one.

shweta__sharma’s picture

shweta__sharma CreditAttribution: shweta__sharma at OpenSense Labs commented

Yes @bnjmnm You are correct I was checking only up-pointing chevron. Down-pointing chevron icon has a 1-2 point px issue.

shweta__sharma’s picture

shweta__sharma CreditAttribution: shweta__sharma at OpenSense Labs commented
Issue summary: View changes

Issue summary Updated. I don't think it has any pending work left. Based on #5 Moving status to RTBC now
Thanks

shweta__sharma’s picture

shweta__sharma CreditAttribution: shweta__sharma at OpenSense Labs commented
Status: Needs work » Reviewed & tested by the community
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Très Bien Tech commented
Status: Reviewed & tested by the community » Needs work

can someone reup a path that applies or better yet a MR?

thx

karanpagare made their first commit to this issue’s fork.

sakthi_dev’s picture

sakthi_dev CreditAttribution: sakthi_dev at Specbee for Drupal India Association commented

The mentioned lines are removed. Is the change really required.

karanpagare opened merge request !6740

karanpagare’s picture

karanpagare CreditAttribution: karanpagare as a volunteer and at QED42 for Drupal India Association commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave CreditAttribution: smustgrave commented
Status: Needs review » Reviewed & tested by the community

Patch https://www.drupal.org/files/issues/2023-08-16/3377198-6.patch copied into the MR.

  • nod_ committed c73225d6 on 11.x 
    Issue #3377198 by karanpagare, Harish1688, kostyashupenko, Vidushi Mehta...

  • nod_ committed 95f98bda on 10.3.x 
    Issue #3377198 by karanpagare, Harish1688, kostyashupenko, Vidushi Mehta...
nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ at Très Bien Tech commented
Status: Reviewed & tested by the community » Fixed

nod_ closed merge request !6740

Status: Fixed » Closed (fixed)

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