Problem/Motivation

This can happen

  • At narrower widths
  • When a dropbutton has more than one word (thus it has a space)

At certain viewport widths, the browser may add a line break between words, which results in the primary button having more height than the disclosure button next to it. The intent is for these heights to match. While this does not make the dropbutton unusable, it is not a visual experience that inspires confidence.

Steps to reproduce

1. Login as an Admin user.
2. Go to a page with a dropbutton that has a primary button with a label that includes at least one space such as "Manage Fields" in Home > Administration > Strucuture > Content types (admin/structure/types).
3. Switch to the tablet device view using inspect tool.

CommentFileSizeAuthor
#25 3321726-nr-bot.txt90 bytesneeds-review-queue-bot
#23 3321726-nr-bot.txt85 bytesneeds-review-queue-bot
#20 3321726_20.patch1.6 KBathyamvidyasagar
#20 before_patch.jpg62.94 KBathyamvidyasagar
#20 after_patch.jpg89.83 KBathyamvidyasagar
#18 After patch.png62.18 KBBushra Shaikh
#18 Before patch.png66.62 KBBushra Shaikh
#12 3321726-afterpatch.jpg112.58 KBgaurav-mathur
#11 afterpatch.jpg27.78 KBgaurav-mathur
#11 3321726-beforepatch.jpg114.73 KBgaurav-mathur
iOS-15.5.png227.88 KBakshaydalvi212

Issue fork drupal-3321726

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

Comments

akshaydalvi212 created an issue. See original summary.

akshaydalvi212 opened merge request !2960

akshaydalvi212 closed merge request !2960

akshaydalvi212 opened merge request !2962

akshaydalvi212’s picture

akshaydalvi212 commented
Status: Active » Needs review
hhvardan’s picture

hhvardan commented
Status: Needs review » Needs work

The change didn't fix the issue on real device because it make changes only for no-touchevent devices.
Probably you need to add the same style to the ".js .dropbutton--multiple .dropbutton__item:first-of-type" selector.

akshaydalvi212 closed merge request !2962

akshaydalvi212 opened merge request !2974

akshaydalvi212’s picture

akshaydalvi212 commented
Status: Needs work » Needs review
gaurav-mathur’s picture

gaurav-mathur commented
Assigned: Unassigned » gaurav-mathur
gaurav-mathur’s picture

gaurav-mathur commented
Assigned: gaurav-mathur » Unassigned
StatusFileSize
new 3321726-beforepatch.jpg114.73 KB
new afterpatch.jpg27.78 KB

The change fix the issue and it is working properlyin all devices. Refer to screenshots. please ignore second image(afterpatch.jpg) it's upload by mistake.

gaurav-mathur’s picture

gaurav-mathur commented
StatusFileSize
new 3321726-afterpatch.jpg112.58 KB

Refer to screenshot.

hhvardan’s picture

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

+1 RTBC

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update

@gaurav-mathur given that the changes are to the Claro theme, and your screenshots are from Olivero, I'm not sure how that before/after could have happened but it's certainly not relevant to this issue

I left feedback in the MR that needs addressing

Also tagging with "Needs issue summary update" as this (seeming correctly) targets.no-touchevents styles in the solution, which suggests it is an issue regarding multiword dropbutton content on narrower withs, and not something specific to tablets/touch devices

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

ameymudras’s picture

ameymudras commented
Status: Needs work » Needs review
Bushra Shaikh’s picture

Bushra Shaikh commented
Assigned: Unassigned » Bushra Shaikh
Bushra Shaikh’s picture

Bushra Shaikh commented
Assigned: Bushra Shaikh » Unassigned
StatusFileSize
new Before patch.png66.62 KB
new After patch.png62.18 KB

Verified patch #16 on Drupal 10.0.x. Patch applied successfully and looks good to me.
The issue is fixed after applying the patch.
refer screenshot:
can be moved to RTBC+1

RTBC+1

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

MR looks good and fixes the issue but this was previously tagged for IS update in #14 which still needs to happen.

athyamvidyasagar’s picture

athyamvidyasagar commented
StatusFileSize
new after_patch.jpg89.83 KB
new before_patch.jpg62.94 KB
new 3321726_20.patch1.6 KB

Drop down issue is fixed for D10. Patch and screenshots are attached.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Title: dropbuttons breaks in Tablet view. » If there is a line break in a primary dropbutton, it will not match the height of the adjacent toggle
Issue summary: View changes
Issue tags: -Needs issue summary update

Hi @akshaydalvi212, are you familiar with Drupal's Gitlab integration? The patch you provided replicates work that is already in this issue's Merge Request, so it doesn't provide any additional value and would not receive credit. The contrib process can be confusing, especially with patches and Merge Requests both being valid ways to provide code changes in an issue. If the docs don't clear things up, it's usually possible to get additional guidance in Drupal slack, particularly if they are targeted questions vs requests for general assistance.

_______________

Issue summary updated.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3321726-nr-bot.txt85 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

sourabhjain’s picture

sourabhjain
Primary language English
commented
Status: Needs work » Needs review

Hidding the #20 patch files as we have already the MR. It is creating confusion.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3321726-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Needs review

The bot switched this to needs work because patch #20 was not passing tests. However, there was already a test-passing merge request awaiting review, so I'm switching it back to that. Be sure to review the MR not any patches (thanks @sourabhjain for hiding those)

And no shade on Needs Review Queue Bot - you are a good and helpful bot.

smustgrave’s picture

smustgrave commented
Version: 10.0.x-dev » 11.x-dev
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

Can the MR be rebased for 11.x and backported

Thanks.

nod_’s picture

nod_
Primary language French
Location Lille
commented

(bot was testing against 10.1.x, not 10.0.x might be why it complained)

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

sagarchauhan opened merge request !4243

sagarchauhan’s picture

sagarchauhan commented
Status: Needs work » Needs review

Rebased the MR for 11.x

smustgrave’s picture

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

Reroll was good.

  • bnjmnm committed 0f2e209b on 11.x authored by sagarchauhan 
    Issue #3321726 by akshaydalvi212, ameymudras, sagarchauhan, bnjmnm: If...
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Reviewed & tested by the community » Fixed

Good simple fix there. Committed to 11.x. Thanks folks!

Status: Fixed » Closed (fixed)

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