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

Made the new tab button touch screen friendly #5927

Merged
merged 1 commit into from
Dec 1, 2016
Merged

Made the new tab button touch screen friendly #5927

merged 1 commit into from
Dec 1, 2016

Conversation

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

Fixes #5925

Auditors: @bsclifton

Test Plan: make sure clicking is expanded enough

@luixxiul
Copy link
Contributor Author

work in progress :-)

@luixxiul
Copy link
Contributor Author

ok, ready for review.

Fixes #5925

Auditors: @bsclifton

Test Plan: make sure clicking is expanded enough
-webkit-mask-image: url('../img/toolbar/newtab_btn.svg');
-webkit-mask-repeat: no-repeat;
-webkit-mask-position: center;
-webkit-mask-size: 12px 12px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the plus image is 12x12 and centered, increasing the height of the button (which is clickable) to 24px.

@bsclifton
Copy link
Member

bsclifton commented Dec 1, 2016

@jkup would you be able to review this?

edit: I'm curious if it'll work as expected with width/height both set, instead of using width and max-height. I remember having to do that because certain resolutions would contort the SVG

@jkup jkup self-assigned this Dec 1, 2016
@jkup
Copy link
Contributor

jkup commented Dec 1, 2016

Yep I'll review now.

@bsclifton bsclifton added this to the 0.13.0 milestone Dec 1, 2016
@bsclifton
Copy link
Member

Awesome! I'll try the Windows version...

@jkup
Copy link
Contributor

jkup commented Dec 1, 2016

++ from me

This approach is cool, I didn't know about it!

Although I'm not really able to repro the initial issue on my Mac so hopefully @bsclifton can merge if the original issue is fixed on Windows!

@bsclifton bsclifton self-assigned this Dec 1, 2016
@bsclifton
Copy link
Member

Looks great 😄 Awesome job, @luixxiul!

Screenshot of the hitbox, so everybody can see. It now extends all the way up and down
image

@luixxiul
Copy link
Contributor Author

Labelled QA/no qa needed as this PR was superseded with #6474

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