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

Don't allow tab preview after close if config is off #9537

Merged
merged 2 commits into from
Jun 19, 2017
Merged

Conversation

cezaraugusto
Copy link
Contributor

@cezaraugusto cezaraugusto commented Jun 17, 2017

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.

Issue first reported in #9306 (comment). cc @jonathansampson

Test Plan:

npm run test -- --grep="webview previews the next tab when current hovered tab is closed"

Manual Test Plan:

  1. Disable tab previews
  2. Open three tabs
  3. Set first as active
  4. Hover over second, close
  5. Third should not be previewed
  6. Enable tab preview again and start from 2.
  7. It should preview

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

Copy link
Collaborator

@jonathansampson jonathansampson left a comment

Choose a reason for hiding this comment

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

Concise. Appears to work. LGTM :)

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Changes look good and test plan works 😄 👍

I did notice though that locally and on travis, one of the tests is failing:
https://travis-ci.org/brave/browser-laptop/jobs/244036926#L3204

Once that is resolved, this is ready for merge 😄

@cezaraugusto
Copy link
Contributor Author

sorry changed frameKey just before pushing. updated.

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Code works great and tests pass! Thanks, @cezaraugusto! 😄

@bsclifton bsclifton merged commit 7d98f45 into master Jun 19, 2017
@bsclifton bsclifton deleted the tabsbar/9536 branch June 19, 2017 04:54
bsclifton added a commit that referenced this pull request Jun 19, 2017
Don't allow tab preview after close if config is off
bsclifton added a commit that referenced this pull request Jun 19, 2017
Don't allow tab preview after close if config is off
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants