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

Adjust min-threshold for closeTab #7799

Merged
merged 1 commit into from
Mar 21, 2017
Merged

Adjust min-threshold for closeTab #7799

merged 1 commit into from
Mar 21, 2017

Conversation

cezaraugusto
Copy link
Contributor

  • 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).

Auditors: @bbondy, @bsclifton

Fix #5431

Note: PR also removed extra gutter for close icon. Now you can only close a tab if you really intent to close a tab ❌ 👍

Test plan:

automated tests must pass:

✓ npm run test -- --grep="should not show closeTab icon if tab size is small and tab is not active"
✓ npm run test -- --grep="should show closeTab icon if tab size is small and tab is active"
✓ npm run test -- --grep="should not show closeTab icon if tab size is extraSmall and tab is not active"
✓ npm run test -- --grep="should show closeTab icon if tab size is extraSmall and tab is active"
✓ npm run test -- --grep="should not show closeTab icon if tab size is the smallest size"

QA steps:

  1. Try to close a tab by hitting an area next to icon (but not the icon itself)
  • Expected: You can't
  1. Have a lot of tabs up until tab is small enough to have two icons almost colliding. (<46px wide)
  • Expected: Active tab should have closeTab icon always visible
  • Expected: Inactive tabs should not have closeTab icon
  1. Have a lot of tabs up until tab is small enough to have just one icon.
  • Expected: Active tab should have closeTab icon always visible
  • Expected: Inactive tabs should not have closeTab icon
  1. Have a lot of tabs up until tab is small enough so favicon is resized to a smaller width
  • Expected: Both active/inactive tabs should not have closeTab icon

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.

Tried this out before lunch- works great! Definitely has my approval. I'll add @bradleyrichter to this PR and we can review here in a few minutes

@srirambv
Copy link
Collaborator

Test 1 fails as the close button is overlapped on the right side of favicon. And the click area is more than the close button causing it to close the tab when clicked on the favicon
newtabtext

@bsclifton bsclifton merged commit b718a40 into brave:master Mar 21, 2017
@luixxiul
Copy link
Contributor

on Windows 10 32 bit I experience exactly the same thing.

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.

Disable close button on tabs if tab size is too small
4 participants