Problem/Motivation

#1:
Overlapping contextual links

Unfortunately this is a fairly obvious user-facing bug. It'd be awesome if there were some way to fix it tomorrow during the QA sprint.

Proposed resolution

Thanks so much @webchick!

+++ b/core/themes/bartik/css/layout.css
@@ -32,7 +32,7 @@ body,
+.region-secondary-menu .block-menu {
...
   overflow: hidden;

This is likely the cause of the first issue. Overflow hidden is ways that case. And likely that class is providing a clearfix which can be replaced by removing it (if the clearfix is no longer necessary) or convering it to a clearfix:

.region-secondary-menu .block-menu:after {
  content: "";
  display: table;
  clear: both;
}

Remaining tasks

Create a patch
Manual testing

User interface changes

API changes

Follow-up to #1869476: Convert global menus (primary links, secondary links) into blocks

CommentFileSizeAuthor
#8 fix-contextual-link-clipping.patch981 bytesskippednote
#7 fix-contextual-link-clipping.patch972 bytesskippednote
#4 fix-contextual-link-clipping.patch972 bytesskippednote

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Title: Convert global menus (primary links, secondary links) into blocks » Follow-up: Convert global menus (primary links, secondary links) into blocks

The issue title majorly WTF-ed me today... :-)

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Thanks should clone issues as follow ups that late at night. Whoops

star-szr’s picture

star-szr
he/him
Primary language English
commented
Title: Follow-up: Convert global menus (primary links, secondary links) into blocks » Fix contextual links on new primary links/secondary links blocks

Suggested re-title :)

skippednote’s picture

skippednote commented
StatusFileSize
new fix-contextual-link-clipping.patch972 bytes

Since the content is in a block level element I don't think there is a need to float and clearfix the navigations. Simple text-align fixes the problem here while reducing the code smells.

Only local images are allowed.

Only local images are allowed.

skippednote’s picture

skippednote commented
Status: Active » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Needs work

That sounds sensible, but I'm no CSS expert, so I can't approve this. This issue is already tagged with "CSS" so I'm sure it'll be reviewed shortly.

In the mean time, there's one detail that's wrong:

+++ b/core/themes/bartik/css/style.css
@@ -657,13 +657,13 @@ body:not(:target) .region-primary-menu .menu-toggle-target-show:target ~ .menu l
-  float: right; /* LTR */
+  text-align: right;

Please keep the /* LTR */ annotation.

skippednote’s picture

skippednote commented
StatusFileSize
new fix-contextual-link-clipping.patch972 bytes

Add /* LTR */ back.

skippednote’s picture

skippednote commented
StatusFileSize
new fix-contextual-link-clipping.patch981 bytes

Had added the same patch.
Re-uploading with /* LTR */ .

skippednote’s picture

skippednote commented
Status: Needs work » Needs review
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing, -needs screenshots

Thanks @skippednote that does the trick.

Screenshots in #4 and I manually tested rtl ltr on simplytest.me.

trogels’s picture

trogels commented

Reviewed and tested #8 patch. Looks good.

NBZ4live’s picture

NBZ4live commented

Marked #2349781: Quick edit context menu not visible in the User account menu in Bartik as duplicate of this issue

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: fix-contextual-link-clipping.patch, failed testing.

Status: Needs work » Needs review

joelpittet queued 8: fix-contextual-link-clipping.patch for re-testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Back to RTBC, Testbot
Was at the "coffeeshop"

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed d21cea7 and pushed to 8.0.x. Thanks!

  • alexpott committed d21cea7 on 8.0.x 
    Issue #2346515 by skippednote | joelpittet: Fix contextual links on new...

Status: Fixed » Closed (fixed)

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