Skip to content
This repository has been archived by the owner. It is now read-only.

Windows only - Bookmarks toolbar - Bookmark hover targets are pushed down if there are no tabs below the folder #9088

Closed
alexwykoff opened this Issue May 27, 2017 · 26 comments

Comments

Projects
None yet
10 participants
@alexwykoff
Copy link
Member

alexwykoff commented May 27, 2017

  • Did you search for similar issues before submitting this one?
    Yes

  • Describe the issue you encountered:
    A user from support reported that the highlight effect did not work as expected if there wasn't a tab underneath the bookmark folder.

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Window

  • Brave Version (revision SHA):
    0.15.310

  • Steps to reproduce:

    1. Create quite a few bookmark folders and add bookmarks
    2. Check bookmark folders above a tab vs those which exist over no tab.
    3. Slowly hover over each option
  • Actual result:
    The bookmark or sub-folder did not highlight for folders without tabs beneath them. (It was later discovered the targets were pushed down to the last couple pixels of each first bookmark entry in the expansion menu.

  • Expected result:
    The bookmark or sub-folder should have a highlight style applied

  • Will the steps above reproduce in a fresh profile? If not what other info can be added?
    Yes

  • Is this an issue in the currently released version?
    Yes

  • Can this issue be consistently reproduced?
    Yes

  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:

  • Any related issues:

@alexwykoff alexwykoff added the bug label May 27, 2017

@alexwykoff alexwykoff added this to the 0.16.200 milestone May 27, 2017

@xjmzx

This comment has been minimized.

Copy link

xjmzx commented May 30, 2017

Have a somewhat similar problem since last update to 0.15.310 on MacOS.

Bookmarks can no longer be dragged, dropped. Only created or deleted.

OS: Sierra 10.12.5 (16F73)

@eljuno

This comment has been minimized.

Copy link
Contributor

eljuno commented Jun 3, 2017

@luixxiul luixxiul added the OS/Windows label Jun 4, 2017

@eljuno

This comment has been minimized.

Copy link
Contributor

eljuno commented Jun 6, 2017

@alexwykoff alexwykoff assigned alexwykoff and unassigned alexwykoff Jun 13, 2017

@alexwykoff alexwykoff modified the milestones: 0.19.x (Nightly Channel), 0.18.x (Developer Channel) Jun 13, 2017

@alexwykoff alexwykoff self-assigned this Jun 13, 2017

@alexwykoff

This comment has been minimized.

Copy link
Member Author

alexwykoff commented Jun 13, 2017

@alexwykoff try to repro this plz 😸

@alexwykoff alexwykoff modified the milestones: 0.20.x (Developer Channel), 0.19.x (Beta Channel) Jul 18, 2017

@eljuno

This comment has been minimized.

@NejcZdovc

This comment has been minimized.

Copy link
Member

NejcZdovc commented Aug 15, 2017

+3 from #10323

@alexwykoff alexwykoff modified the milestones: 0.21.x (Nightly Channel), 0.20.x (Developer Channel) Aug 15, 2017

@eljuno

This comment has been minimized.

Copy link
Contributor

eljuno commented Aug 15, 2017

@eljuno

This comment has been minimized.

@luixxiul

This comment has been minimized.

Copy link
Contributor

luixxiul commented Oct 19, 2017

I suspect it is due to that the area is reserved for system context menu on Windows. @bsclifton might know well about that.

@bbondy bbondy modified the milestones: 0.21.x (Developer Channel), 0.20.x (Beta Channel), Backlog Oct 25, 2017

@bbondy bbondy modified the milestones: Triage Backlog, Prioritized Backlog Nov 2, 2017

@bsclifton

This comment has been minimized.

Copy link
Member

bsclifton commented Dec 4, 2017

@jonathansampson

This comment has been minimized.

Copy link
Collaborator

jonathansampson commented Dec 4, 2017

This does appear to rely on the mouse being within the region of the tab/new-tab elements below:

hit-regions
hit-regions-border

Looks like it may be identical/related to #9436 (comment).

@jonathansampson

This comment has been minimized.

Copy link
Collaborator

jonathansampson commented Dec 4, 2017

The problem here is collision with drag regions. These do not respect z-index, and thus take-over the mouse anytime you enter a the coordinates of a drag-region. In the above example, the entire tabStrip is draggable. The tabs and new-tab button are not, which allows you to hover bookmarks while in their drag region.

One solution is to temporarily remove the .allowDragging class from .tabStripContainer when a bookmark menu is opened, and restore the class once the bookmark menu has closed.

<span class="tabStripContainer allowDragging">...</span>
@LaurenWags

This comment has been minimized.

Copy link

LaurenWags commented Dec 22, 2017

@eljuno

This comment has been minimized.

@srirambv

This comment has been minimized.

@eljuno

This comment has been minimized.

Copy link
Contributor

eljuno commented Jan 18, 2018

@NejcZdovc

This comment has been minimized.

Copy link
Member

NejcZdovc commented Jan 20, 2018

I think that we need to bump priority on this one @bsclifton

@eljuno

This comment has been minimized.

Copy link
Contributor

eljuno commented Jan 23, 2018

@NejcZdovc NejcZdovc modified the milestones: Backlog (Prioritized), 0.21.x (Developer Channel) Jan 23, 2018

@NejcZdovc

This comment has been minimized.

Copy link
Member

NejcZdovc commented Jan 23, 2018

pulled into 0.21

@eljuno

This comment has been minimized.

Copy link
Contributor

eljuno commented Feb 8, 2018

Can't reproduced on latest stable version 0.20.30

Record:
first-bookmark-20-30

@jonathansampson

This comment has been minimized.

Copy link
Collaborator

jonathansampson commented Feb 13, 2018

@elia The dropdown needs to overlap the tab and new-tab button. See the positions of the UI elements in this comment: #9088 (comment)

@NejcZdovc

This comment has been minimized.

Copy link
Member

NejcZdovc commented Feb 14, 2018

pushing back to 0.22

@NejcZdovc NejcZdovc modified the milestones: 0.21.x (Beta Channel), 0.22.x (Developer Channel) Feb 14, 2018

@eljuno

This comment has been minimized.

Copy link
Contributor

eljuno commented Feb 14, 2018

@jonathansampson Still can't reproduce this on latest stable. 😄

first-bookmark-20-30-overlap

@bbondy bbondy modified the milestones: 0.22.x (Developer Channel), 0.23.x (Nightly Channel) Feb 25, 2018

@alexwykoff

This comment has been minimized.

Copy link
Member Author

alexwykoff commented Mar 13, 2018

@bsclifton @jonathansampson feel free to close if no repro thx

@alexwykoff alexwykoff added priority/P4 and removed priority/P5 labels Mar 20, 2018

@bsclifton

This comment has been minimized.

Copy link
Member

bsclifton commented Mar 20, 2018

Confirmed this works great using 0.21.18 on Windows 10 x64 😄 👍

@bsclifton bsclifton closed this Mar 20, 2018

@bsclifton bsclifton removed this from the 0.23.x (Developer Channel) milestone Mar 20, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.