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

fix pinning topSites not respecting position #5645

Merged
merged 1 commit into from Nov 15, 2016
Merged

Conversation

@cezaraugusto
Copy link
Contributor

cezaraugusto commented Nov 15, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Ran git rebase -i to squash commits (if needed).

Auditors: @bsclifton

fix #5337

Test Plan:

  • Access new tab page;
  • Access 4-6 websites;
  • Pin the second and fourth site.;
  • Pinning a site should add a Pin icon on top-right corner and be kept on its position (not jump);
  • Access another website (i.e. https://brave.com) and get back to new tab.
  • Ensure that pinned topSites are still on their positions (2nd and 4th);
  • Ensure that your latest accessed site is on first position;
  • Ensure that other sites were pulled to right, but skipping pinned positions.
Auditors: @bsclifton

fix #5337

Test Plan:

* Access new tab page;
* Access 4-6 websites;
* Pin a site. Pinning a site should add a Pin icon on top-right corner and be kept on its position (not jump);
* Keep in mind the website position and reorder it to a new position;
* Access another website and get back to new tab. Ensure that your last pinned website kept its position;
@bsclifton
Copy link
Member

bsclifton commented Nov 15, 2016

LGTM 😄

@bsclifton bsclifton merged commit dd20048 into brave:master Nov 15, 2016
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
@cezaraugusto cezaraugusto deleted the cezaraugusto:feature/newtab/5337-2 branch Nov 15, 2016
@cezaraugusto cezaraugusto mentioned this pull request Nov 16, 2016
3 of 3 tasks complete
@srirambv
Copy link
Collaborator

srirambv commented Nov 21, 2016

Ensure that your latest accessed site is on first position

visiting a new site puts the tile in between the two pinned tiles and not in first position.

ezgif com-resize

@cezaraugusto
Copy link
Contributor Author

cezaraugusto commented Nov 21, 2016

@srirambv could you open a new issue for that? This is more related to how we define topSites (based on number of visits). On your comment pinned topSites are still on their positions, so slightly different. Thanks!!

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

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.