I think Tim said this is actually a Bartik issue.

However, this looks rather assy. :P

Edit buttons overlap the table cell

Environment details to reproduce the issue :

Chrome Version 52.0.2743.116 (64-bit)
Mac OsX

CommentFileSizeAuthor
#39 2781579-39.patch485 bytestedbow
#34 Drupal-8-firefox.png21.57 KBVidushi Mehta
#34 drupal-8-chrome.jpeg40.27 KBVidushi Mehta
#33 2781579-33.patch483 bytesamit.mall
#30 2781579.jpg65.31 KBamit.mall
#29 2781579-fix-for-text-overlapping.patch503 bytesamit.mall
#20 2781579-20.patch776 bytesVidushi Mehta
#19 drupal 8.3-chrome-after.jpeg116.38 KBVidushi Mehta
#19 drupal 8.3-ff-after.png45.45 KBVidushi Mehta
#19 2781579-19.patch494 bytesVidushi Mehta
#19 drupal 8.3-chrome-before.jpeg74.11 KBVidushi Mehta
#19 drupal 8.3-ff-before.png26.58 KBVidushi Mehta
#17 Screen Shot 2016-09-28 at 15.05.42.png25.58 KBnathanlawsn
#16 Configure Block moonpeak.png40.57 KBmoonpeak
#14 2781579-12.patch1.23 KBbrahmjeet789
#13 2781579-12.patch1.23 KBbrahmjeet789
#11 2781579-11.patch732 bytesnathanlawsn
#10 drupal8-home.png60.69 KBManjit.Singh
#9 d8.png39.12 KBVidushi Mehta
#9 2781579-9.patch494 bytesVidushi Mehta
#8 Outside-in-width-1280.png253.21 KBnaveenvalecha
#8 Outside-in-width-1687.png220.18 KBnaveenvalecha
#5 Outside-in-Tools-block-edit.png73.85 KBnaveenvalecha
#4 outside-in.png41.32 KBManjit.Singh
Screen Shot 2016-08-08 at 3.44.04 PM.png72.66 KBwebchick
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick created an issue. See original summary.

webchick’s picture

Issue tags: +Outside-in, +Usability, +sprint
Manjit.Singh’s picture

Assigned: Unassigned » Manjit.Singh
Manjit.Singh’s picture

Assigned: Manjit.Singh » Unassigned
Status: Active » Needs review
FileSize
41.32 KB

Looks like this issue is exist only for mac users, I have checked it in Linux-Ubuntu, It looks good to me. Buttons are not overlapping with the scrollbar.

ousidein

naveenvalecha’s picture

Issue summary: View changes
Status: Needs review » Active
FileSize
73.85 KB

This does not seems to be the OS specific rather than browser version specific.
I'm able to reproduce it.
Adding screenshot with the environment detail.

Manjit.Singh’s picture

I am not able to see the screenshot @Naveen. Seems like it has nothing.

Gábor Hojtsy’s picture

Issue summary: View changes

@naveenvalecha: yeah it has a dot and a red dashed square, hard to tell what you mean by that

naveenvalecha’s picture

FileSize
220.18 KB
253.21 KB

Sorry for the wrong snapshot. Seems the Awesome Screenshot chrome extension was not happy that time :P

I investigated the problem more in depth, the problem is not macspecific. its due to the resolution of the screen.See in the screenshots below.I'll dig further with Manjit tomorrow in person.

Vidushi Mehta’s picture

Status: Active » Needs review
FileSize
494 bytes
39.12 KB

I have attached patch for the above issue with screenshot. Please review.

Manjit.Singh’s picture

Status: Needs review » Needs work
Issue tags: +Dublin2016
FileSize
60.69 KB

Seems like text is still overlapping and not visible properly. see the screenshots.

erwqe

nathanlawsn’s picture

Status: Needs work » Needs review
FileSize
732 bytes

Seems that there is insufficient space when the row weights are visible.

If you remove the 120px minimum width from the first column and set the table row & column width to 100% it has sufficient space.

Don't suppose you know why we have a minimum width?

Status: Needs review » Needs work

The last submitted patch, 11: 2781579-11.patch, failed testing.

brahmjeet789’s picture

brahmjeet789’s picture

please review the patch

brahmjeet789’s picture

Status: Needs work » Needs review
moonpeak’s picture

FileSize
40.57 KB

Hi,

Could you specify the steps to reproduce the issue. I downloaded and applied the patch on my local instance but, was unable to see menu under block layout.

nathanlawsn’s picture

@moonpeak Have you enabled the Settings Tray module?

If so, hit 'Quick Edit' on the Tools block.

Quick Edit option under Contextual filters

Manjit.Singh’s picture

Version: 8.2.x-dev » 8.3.x-dev
Status: Needs review » Needs work

This need bit more testing with the latest patch. @moonpeak It would be good if you can do that.

Vidushi Mehta’s picture

Added new patch for this with before and after screenshots. @Manjit please look into this.

Vidushi Mehta’s picture

This is the updated patch. Please review this.

Vidushi Mehta’s picture

Gábor Hojtsy’s picture

@Vidushi Mehta: why is this better/different, what did you find with the previous one, how is this fixing it better?

Manjit.Singh’s picture

Status: Needs review » Needs work

@Vidushi: Seems like #19 & #20 have completely different from each other. Seems like, there some mistake while creating the patch file. Can you please look into this. And Please don't forget to attach interdiff file in future.

Vidushi Mehta’s picture

@Gabor and @Manjit sorry actually by mistake i had uploaded my old patch that is #19 but the new and correct one is #20 and the screenshots that i had attached with #19 are for the patch #20.
So please ignore #19 patch and review #20.

Vidushi Mehta’s picture

Status: Needs work » Needs review
tkoleary’s picture

Issue tags: -sprint, -Dublin2016
tkoleary’s picture

Status: Needs review » Closed (cannot reproduce)

Other changes have resolved this issue by introducing better sandboxing of tray css.

tkoleary’s picture

Status: Closed (cannot reproduce) » Needs work

No, I spoke too soon. The problem at #10 still exists, but the patch at #20 does not properly resolve it.

What is needed is a way to make the table 100% but for it to 'flex' when the extra scroll bar appears.

amit.mall’s picture

I have fixed text "Operations" overlapping issue and it's working fine. I have attached patch for this please review.

amit.mall’s picture

FileSize
65.31 KB
tkoleary’s picture

Status: Needs work » Reviewed & tested by the community

Looks great!

xjm’s picture

Status: Reviewed & tested by the community » Needs review

Thanks @amit.mall!

Doesn't this just permanently reduce the width, though? Is there a particular reason 85px is being chosen?

Also, there's this CSS just above:

.ui-dialog-offcanvas td a {
  display: block;
  max-width: 120px;
  overflow: hidden;
}
.ui-dialog.ui-dialog-offcanvas tr td:first-child,
.ui-dialog.ui-dialog-offcanvas tr th:first-child {
  min-width: 120px;
  padding-left: 20px; /* LTR */
  text-overflow: ellipsis;
  white-space: nowrap;
  overflow: hidden;
}
amit.mall’s picture

@xjm #32, I found 85px is minimum width to occupy left side content, but since there is a css property- white-space: nowrap; so no need to have min-width, I have removed this in new attached patch, result is same

Vidushi Mehta’s picture

FileSize
40.27 KB
21.57 KB

I reviewed patch #33. Looks fine for me.
Attaching screenshots for the same after applying the patch.

tkoleary’s picture

Component: Bartik theme » outside_in.module
tkoleary’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Needs work

Needs a re-roll after one of the ones that was committed today.

webchick’s picture

In the meantime, adding credit.

tedbow’s picture

Status: Needs work » Needs review
FileSize
485 bytes

Just a re-roll

tkoleary’s picture

Status: Needs review » Reviewed & tested by the community

  • webchick committed cdb2ecd on 8.3.x
    Issue #2781579 by Vidushi Mehta, amit.mall, brahmjeet789, nathanlawson91...
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

  • webchick committed 1c20c89 on 8.2.x
    Issue #2781579 by Vidushi Mehta, amit.mall, brahmjeet789, nathanlawson91...

  • webchick committed cdb2ecd on 8.4.x
    Issue #2781579 by Vidushi Mehta, amit.mall, brahmjeet789, nathanlawson91...

  • webchick committed cdb2ecd on 8.4.x
    Issue #2781579 by Vidushi Mehta, amit.mall, brahmjeet789, nathanlawson91...

Status: Fixed » Closed (fixed)

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

tedbow’s picture

Component: outside_in.module » settings_tray.module

Changing to new settings_tray.module component. @drpal thanks for script help! :)