Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Tabs transitions / drag & drop / realtime tear-off #11720

Closed
wants to merge 16 commits into from

Conversation

petemill
Copy link
Member

@petemill petemill commented Oct 30, 2017

This is a work in progress, but very much ready for both design and engineering feedback

Fix #6242
Fix #9211
Fix #557
Closes #8459 due to feature removal (no mouse cursor icon now)
Possibly addresses #11207

Dependent on / do not merge before:

#12438 (Buffer Windows)
#12492 (ReduxComponent can accept any function for mergeProps)
#12373 (Standard validates flow types)
#12371 (transform-class-properties babel plugin)
#12116 (--debug-tab-events #flag)

Main features

Dragging tabs to sort

  • Dragging the tab moves the tab to follow the mouse, fixed to the axis of the tab bar
  • As the edge of a tab passes the middle of another tab, it swaps positions with that tab

Smooth transitions for tabs

  • Opening a tab
    • Animate Y from 50% to 100%
    • Duration 710ms
  • Closing a tab
    • Animate Y from 100% to 0
    • Duration 710ms
  • Re-ordering a tab
    • Slight staggered delay of 30ms (if moving multiple positions, from a single continuous drag)
    • Animation duration of 710ms
    • Ease Out

Detaching a tab from a window

  • Threshold of 44px on Y axis when the tab hasn't yet been dragged past another tab horizontally
  • Threshold increases to 90px on Y axis when the tab has been dragged past another tab horizontally
  • Threshold also always requires mouse has been outside that threshold for 300ms (meaning the mouse can drag very far away and back within 300ms and it wil not trigger a detach)
  • Detaches whilst drag is still happening
  • New window follows mouse until mouse is let go, ESC is pressed, or tab is dragged to another window's tab bar for attachment

Attaching a tab to a window

  • Dragging a tab in a window with only one tab, moves the whole window
  • When it approaches the tab bar of another window, it is attached to that window

Issues

Done

  • Tabs don't animate 'appear' when switching between pages
  • Tab content which has own animation (favicon, title, audio icon, etc) don't restart animations when tab is moved
  • Moving animation does not restart when tab is dragged back and forth, before previous animation completes
  • Ensure 60fps, at least in original window where drag originates

Remaining

Major

  • If the user starts tab dragging from a single-tab window and then attaches the tab to another window, the dragging will stop and the user will need to start dragging (with a new click) the tab in its new location again in order to re-sort the tab in the new window. This is because the event is controlled from the originating window, which gets destroyed at that point. So this isn't an issue if the drag originates in a window with multiple tabs - you can attach, sort, detach from as many windows as you can in one single movement if that's the case. This can be fixed by having the window hanging around until the drag is ended, or using setTimeout and BrowserWindow.sendInputEvent to proxy mousemove in to that window (that would also solve the ubuntu problem)
  • Tab pages. The idea is that the user should be able to drag to an arrow and the tab moves to the previous / next page. This might feel jarring perhaps, because if you drag to the 'previous page' button and the tab moves to the previous page - making the currently displayed page change - the mouse will still be over the 'previous page' button, which would maybe cause an expectation that the page would keep moving to the previous one until it ends up at the first one... It could just be that mousing over the previous / next button only moves one page until a mouse-out and mouse-in occurs again, but if user pauses on the prev/next page button then keep changing the page index. We need to solve this otherwise there'll be no way to change tab pages for a tab unless using a keyboard shortcut. (plan ok'ed by @bradleyrichter)

Medium

  • (Hide windows until they are either rendered or a timeout expires #11764) Speed of window creation. Could create a buffer window when any drag operation starts, so it's rendered hopefully by the time a tab is detached, or just wait for the window to be fully rendered before showing, avoiding flash-of-white (separate issue?)
  • Linux/Ubuntu - not getting mousemove outside window unlike macOS and Windows, so detach not working so well. Could try solving with setTimeout and BrowserWindow.sendInputEvent browser-side, which could also solve the 1-tab-window problem above.
  • nicer visual appearance of detached window
    - minimal top UI?
    - transparency? (electron supports it, muon does not - transparent frameless windows have stopped working muon#373)
    - remove pinned tabs
  • A detached tab should be on the same position in the X-axis as it was in the source window, so that the detached window has exactly the same position.
  • Cancellation. When you press escape, the tab should go back to it's original position / window. The plumbing is set up for this, so should be relatively low effort

Minor

  • Fix tab borders
  • Ghost Tab flash when detaching
  • Always move detached window to the mouse cursor, when it shows (and is ready)
  • Tab position jumps around initially when attaching
  • make new attached tab active
  • first run new window opens underneath macOS menu bar
  • Tab does not retain its size from the source window when attached to another window. The main use-case here is that a user may drag a wide tab into a window with many tabs, and then the tab becomes narrower. It may feel as if you are dragging the space next to the tab.
  • Disable navigation bar mouseout / mouseenter handlers so that url bar (and others) does not flash between states
  • Consider not allowing portion of windows to be dragged outside entire screen bounds during a tab-drag, since some platforms don't allow it anyway
  • Tab attachment works even to a window that is completely hidden behind other windows. They become top-most when the mouse moves near them. This can be solved by listening to mouseenter on the tab strip to detect when the tab is 'dragged' to a different window's tabstrip.
  • Pinned tabs - offset the position of the non-pinned tab dragging when there are pinned tabs present.
  • You can no longer drag a tab from non-pinned to pinned status or the opposite. This is similar to how dragging in other browsers work. Plus it doesn't work if there are not already pinned tabs already. And since pinned tabs are multi-window, it could break the connection between moving a physical single-tab and what ends up happening. If this is important, I'll possibly have to re-work quite a bit.
  • Remove process.stdout.write debug logging
  • When detaching, the tab disappears before the new window shows. This is a disconcearting UX. The ideal here is to overlay the new window, and tab immediately, before the tab disappears from the existing window. This will be improved when single-webview-rendering is available, as it will be much quicker to attach a new tab, or if we can figure out how to render a guest in a very low-cost 'preview' webview whilst waiting for the main webview in the new window to perform the attach.
  • When attaching, the tab flashes from the end of the tab strip, to the position of the mouse (if the mouse button is still down). It should immediately be attached to the position corresponding to the mouse position
  • linux not getting mouse events outside window area. this can effect single-tab window movement when the mouse moves faster than the window does, and ends up outside the window - it 'loses' the window tracking. Also means detach has to happen within the bounds of a window. But we are getting mouseup outside the window, so we can do mouse tracking with node through browser process and setTimeout, as drag is still successfully ended on mouseup
  • Regression 11/28 - dragging to attach a tab to a window on page that isn't the last page switches the page to the last page and finishes the drag operation prematurely
  • perf - render order for current group of tabs should be based on frameKey order, and use the flex 'order' property for display order, resulting in fewer DOM changes on sort
  • When detaching, the new frame in the new window does not have an index. This prevents dragging another tab to that window.
  • macOS - during single-tab window drag, on other window mouseenter, dispatch an action to focus the window
  • If dragging to attach to a window creates a new page, the tab is correctly dragged to the page that was visible, but the page is switched to the last one

Engineering notes

  • Uses the FLIP technique for checking to see where a component's DOM node has been rendered, comparing it to where it was previously rendered (when props last changed), setting the node's position back to the last position and animating it to the new position. https://aerotwist.com/blog/flip-your-animations/

  • The component ListWithTransitions is used to perform these animations on component enter / leave / move. It has nothing to do with dragging, and everything to do with responding to re-ordered tabs or new tabs or closing tabs, and animating those states. I've tried to keep it agnostic as to what is being rendered, so it can be used for other things in the future (e.g. bookmarks).

  • The existing Tab component can do two things:

    1. Initiate a tab dragging via the normal DOM onDragStart event.
      • Dispatches an event informing which tab is being dragged
        • windowStore sends messages to other windows when the mouse moves (see the code comments for why)
    2. Respond to the state instructing a specific tab to be 'dragged'
      • Listens for mousemove in order to move the tab with the mouse, dispatch actions to re-order the tabs, or detach the tab from the window

    Both these things can actually happen in separate components and windows during a drag operation, since a drag may start inside a component in one window, then move the tab to another window, where a different component and window will need to know to be props.isDragging = true, and listen for the mouse position.

  • windowActions vs appActions:
    During a drag operation, it's required to dispatch several actions to the store. The most frequent is to change the display index for the dragged tab. It's important to have the operation complete within max 16ms to maintain 60fps. Whilst the browser process handles changing the tab index, the renderer process must be used to get the correct display index for the frame... (AFAIK), therefore this command gets dispatched by the <Tab /> component, then the correct destination frame index processed by the window store, then the final tab move dispatched to the app store. Ideally we could cut out the middle, and the <Tab /> would be able to communicate which exact index is required direct to the appStore. I've used a windowAction for the first dispatch since appActions can take over 70ms to reach the windowStore. It's still not ideal as windowActions take between 2 - 20ms to get processed, but it's mostly smooth enough now.

Implementation captures

Tab sorting within a single tab-page

sort-single-page

Tab sorting between tab-pages

sort-between-pages

Detach a tab to its own window

detach-simple

Attach a single-tab window to another window

attach-single-page

Attach and detach and attach!

attach-and-detach-and-attach

QA

Manual Tests
https://github.com/petemill/browser-laptop/wiki/Tab-dragging,-sorting,-detaching-and-attaching

Automated Tests
TODO

@petemill petemill added design A design change, especially one which needs input from the design team. feature/tabsbar feature/tear-off-tabs product/user-happiness labels Oct 30, 2017
@petemill petemill self-assigned this Oct 30, 2017
@petemill petemill modified the milestone: Backlog Oct 31, 2017
@petemill petemill force-pushed the tabs-transitions branch 5 times, most recently from 926a744 to 9e63bc8 Compare November 3, 2017 03:34
@brave brave deleted a comment from codecov-io Nov 6, 2017
@petemill petemill mentioned this pull request Nov 7, 2017
8 tasks
@brave brave deleted a comment from codecov-io Nov 7, 2017
@petemill petemill force-pushed the tabs-transitions branch 2 times, most recently from f3513a8 to f33d124 Compare November 8, 2017 05:40
// Set background-color and color to active tab and private tab
this.props.isActive && styles.tabArea__tab_active,
this.props.isPrivateTab && styles.tabArea__tab_private,
(this.props.isPrivateTab && this.props.isActive) && styles.tabArea__tab_private_active,

Copy link
Contributor

@luixxiul luixxiul Nov 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's keep the blank lines above to make those comments meaningful and understandable at a first glance :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst it highlights this specific area, it may harm the ability to scan of the rest of the render structure because typically blank lines should be used for component separation, and not usually in the middle of a component definition. I would suggest that the comments themselves act as padding between the lines of code, to highlight their importance or grouping. But this was your code so I’ll add the spacing back before I’m done. Thanks

@bsclifton bsclifton modified the milestones: 0.21.x (Beta Channel), 0.22.x (Developer Channel) Feb 15, 2018
@bsclifton
Copy link
Member

bsclifton commented Feb 15, 2018

Updated PR and all issues to be in 0.22.x milestone since we're focusing on the single webview changes for 0.21.x 😄

@bbondy bbondy modified the milestones: 0.22.x (Developer Channel), 0.23.x (Nightly Channel) Feb 25, 2018
@petemill petemill changed the title Tabs transitions Tabs transitions / drag & drop / realtime tear-off Feb 27, 2018
petemill and others added 16 commits May 17, 2018 16:42
Tab animations for appearing, removing and changing order.

Platform differences in window behavior with regard to inter-window mouse
event communication requires handling window moves and detection of dragging
over a tab bar in a new window differently in each platform.
…low by default.

Overflow was enabled when tab preview styles showed the tab 'growing' larger than the tabs toolbar
@bsclifton
Copy link
Member

Closing - @petemill evaluated if we could pull this in and unfortunately it would take 4-6 days to complete (and we'd get this for free with Brave Core)

@bsclifton bsclifton closed this Jun 4, 2018
@bsclifton bsclifton removed this from the 0.23.x (Developer Channel) milestone Jun 4, 2018
@bsclifton bsclifton deleted the tabs-transitions branch August 18, 2018 05:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants