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

Dragging tabs no longer shows tabs physically moving #6033

Closed
bsclifton opened this issue Dec 6, 2016 · 13 comments
Closed

Dragging tabs no longer shows tabs physically moving #6033

bsclifton opened this issue Dec 6, 2016 · 13 comments

Comments

@bsclifton
Copy link
Member

Did you search for similar issues before submitting this one?
YEs

Describe the issue you encountered:
Tabs used to show an animation of the surrounding tabs moving out of the way:
tabs moved

Somewhere along the way, this regressed.
Per @bradleyrichter:
This was in 0.12.1 and it fell off shortly after that. By 0.12.5 it was gone

Expected behavior:
Should behave as they do in the picture

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

  • Brave Version: 0.12.5+

  • Steps to reproduce:

    1. Drag tab over another tab
    2. Expect to see tabs move out of the way, so you know where it'll be placed
    3. Be disappointed ☹️
@bsclifton bsclifton added accessibility design A design change, especially one which needs input from the design team. labels Dec 6, 2016
@bsclifton bsclifton added this to the 0.13.1 milestone Dec 6, 2016
@bsclifton
Copy link
Member Author

bsclifton commented Dec 6, 2016

I narrowed it down; great memory, @bradleyrichter 😄

  • This behavior was present in 0.12.1 (f51a293)

  • This behavior was broken by 0.12.2 RC1 (dfe352c)

(any help narrowing it down further is appreciated!)

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Dec 10, 2016

I tried to reproduce this on my computer, but when I went back in time I get the following error:

Module version mismatch. Expected 50, got 49

I tried with node rebuild to version 1.4, which solved this problem, but tabs were not behaving as are on your screenshot in version 0.12.1 (f51a293). I don't know if this is because of rebuild or something else.

@bsclifton
Copy link
Member Author

@NejcZdovc interesting- did you clear your node_modules and redo the npm install?

I was able to narrow down the above not by using source, but by downloading/installing releases (and working off the commits tied to them)

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Dec 10, 2016

@bsclifton yes I deleted my node_modules even electron folder, clean install and even clean pull, but I always get this error. When I switched back to the master branch and run npm_install everything was working correctly out of the box.

I am using node version 7.2.0 and npm version 4.0.3. I would expect that I would get a different error something like expected 50, got 52, because I was going back to September. But it's the other way around which is strange.

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Dec 10, 2016

I would love to tackle this problem, but I need to make it work first somehow 😃

@bsclifton
Copy link
Member Author

bsclifton commented Dec 10, 2016

You may try installing an older version of node.js; I believe back then, we were using version 6.3. Because building our fork of electron involves compiling node.js, it may need to be on the same version (although, it shouldn't matter for this; since it should be using pre-built)

@NejcZdovc
Copy link
Contributor

Will try with node version switcher to set it to 6.3, hope this will help

@jonathansampson
Copy link
Collaborator

This was recently mentioned on Twitter by a user:

Rearranging pinned tabs feels weird, like they're not really moving. Chrome's animations are a good feedback as I'm dragging them that something is happening. With Brave I just have to trust that letting go will drop the pinned tab where I want it.
https://twitter.com/jamonholmgren/status/821984857232281600

Dragging tabs in 0.13.0 (Preview 9)

drag-tab

@bsclifton
Copy link
Member Author

@NejcZdovc I know it's been a while- did you want to check this one out?

@NejcZdovc NejcZdovc self-assigned this Jan 20, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jan 22, 2017
Resolves brave#6033

Auditors: @bsclifton

Test Plan:
- try dnd tabs as mentioned in brave#6033
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jan 22, 2017
Resolves brave#6033

Auditors: @bsclifton

Test Plan:
- try dnd tabs as mentioned in brave#6033
@NejcZdovc NejcZdovc mentioned this issue Jan 22, 2017
4 tasks
bbondy added a commit that referenced this issue Jan 22, 2017
@bbondy bbondy modified the milestones: 0.13.0, 0.13.1 Jan 22, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jan 23, 2017
Resolves brave#6033

Auditors: @bsclifton

Test Plan:
- try dnd tabs as mentioned in brave#6033
@luixxiul
Copy link
Contributor

It seems that it does not work as it should

dnd

the tab should be dropped to where 🚫 is displayed.

Also we don't need to have this one:

dnd

@luixxiul
Copy link
Contributor

luixxiul commented Jan 26, 2017

I created the issue as a follow up: #6863

@NejcZdovc
Copy link
Contributor

@luixxiul regarding the second one (space around active tab) I already talked with @bradleyrichter and I will remove it when I will be working on tab animations

@luixxiul
Copy link
Contributor

Sweet!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants