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

bookmark modal animation is incredibly slow #6748

Closed
alexwykoff opened this issue Jan 19, 2017 · 11 comments
Closed

bookmark modal animation is incredibly slow #6748

alexwykoff opened this issue Jan 19, 2017 · 11 comments

Comments

@alexwykoff
Copy link
Contributor

alexwykoff commented Jan 19, 2017

Test plan

  1. Click the star icon on the URL bar
  2. Make sure the bookmark dialog appears without animation

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

Describe the issue you encountered:
When bookmarking a page via the keyboard shortcut, the modal animation is incredibly slow.

Expected behavior:
The modal should open almost instantly.

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Tested on OS X.

  • Brave Version (revision SHA):
    0.13.0 preview 10 (c60b783)

  • Steps to reproduce:

    1. Navigate to a site.
    2. Bookmarks the site via the keyboard combination.
  • Extra QA steps:
    1.
    2.
    3.

  • Screenshot if needed:

  • Any related issues:

@alexwykoff alexwykoff added this to the 0.13.0 milestone Jan 19, 2017
@bbondy
Copy link
Member

bbondy commented Jan 19, 2017

I see your steps to reproduce, and i raise you a "Steps to not reproduce":

  1. Clean your profile
  2. Start the browser
  3. Browse to brave.com
  4. Wait for it to load
  5. Command+D

Actual results:
Fades in fairly fast.

Any idea what I'm doing wrong?

@luixxiul
Copy link
Contributor

luixxiul commented Jan 20, 2017

This should have been solved with #6382 for #6380

@bbondy
Copy link
Member

bbondy commented Jan 20, 2017

The code with .bookmarkHanger doesn't exist anymore there, maybe check the history on the file to see when it was changed out after that.

@bbondy
Copy link
Member

bbondy commented Jan 20, 2017

I don't think this is blocking, and we could use some more info here so I'm going to move to 0.13.1.

@bbondy bbondy modified the milestones: 0.13.1, 0.13.0 Jan 20, 2017
@luixxiul
Copy link
Contributor

Is this not?

screenshot 2017-01-20 13 43 46

@bbondy
Copy link
Member

bbondy commented Jan 28, 2017

You're right @luixxiul I must have been searching the wrong repo or something.

@bsclifton bsclifton added needs-info Another team member needs information from the PR/issue opener. perf labels Jan 31, 2017
@bsclifton
Copy link
Member

With no owner and no solid STR, I don't think this should be included in 0.13.2. Perhaps it is related to the # bookmarks that a user has? (and in that case, our site list => map change should help)

I'm going to mark as contributor backlog and tag as info needed

@bsclifton bsclifton modified the milestones: contributor backlog, 0.13.2 Jan 31, 2017
@luixxiul
Copy link
Contributor

luixxiul commented Feb 1, 2017

For a contributor:

-    > * {
+
+    // Animate all direct descendants except for the bookmark bar
+    // PR #6382
+    > *:not(.bookmarkHanger) {

I think @bkilrain would like to fix the issue. the hint is above ;-)

@luixxiul luixxiul added bug/good-first-bug and removed needs-info Another team member needs information from the PR/issue opener. perf labels Feb 1, 2017
@bkilrain
Copy link
Contributor

bkilrain commented Feb 1, 2017

I'm up for the challenge!

@luixxiul
Copy link
Contributor

luixxiul commented Feb 1, 2017

@bkilrain Sweet! Please don't forget to include Fix #6748 and Fix #6380 (#6380) in the commit message. Thanks in advance!

@luixxiul
Copy link
Contributor

Fixed with #8443 (#8136)

@luixxiul luixxiul removed this from the contributor backlog milestone Apr 30, 2017
@luixxiul luixxiul self-assigned this Apr 30, 2017
@luixxiul luixxiul added this to the 0.15.2 milestone Apr 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.