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

Make tab close button respect tab's container #5480

Closed
wants to merge 1 commit into from
Closed

Make tab close button respect tab's container #5480

wants to merge 1 commit into from

Conversation

cezaraugusto
Copy link
Contributor

@cezaraugusto cezaraugusto commented Nov 8, 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).

Fix #5431

Auditors: @bbondy, @bsclifton

Note:

This change fixed the issue, but affected tab icons on small windows (now being overflowed in a different way). Doesn't look weird imo but can be improved per team's request.

Test Plan:

  • Reduce window's width checking for close button on tabs
  • Close button should not overflow tab container on narrow views, avoiding UI mismatch

@cezaraugusto cezaraugusto added this to the 0.12.9dev milestone Nov 8, 2016
@bbondy
Copy link
Member

bbondy commented Nov 8, 2016

A bit worried this will break some tests at small window sizes

@bbondy
Copy link
Member

bbondy commented Nov 8, 2016

which maybe travis has?

@bbondy
Copy link
Member

bbondy commented Nov 8, 2016

hard to tell because our tests are so broken right now.

@bsclifton
Copy link
Member

bsclifton commented Nov 8, 2016

I think a better way to solve this would be to consider the size of the tab itself

.tabArea:not(.partOfFullPage) could use a min width, so that it never gets smaller than the favicon (28px seems to work nicely)

You can then solve the actual issue w/ .closeTabButton by setting overflow: hidden. It'll auto-hide when the parent isn't big enough to accommodate the button (however, it'll need some playing around, as sometimes the button gets cut off- might need a margin-right)

Since I don't think this should be rushed and it's not critical for next release, I'm going to assign it to 0.12.10

@bsclifton bsclifton modified the milestones: 0.12.10 release , 0.12.9dev Nov 8, 2016
@cezaraugusto cezaraugusto changed the title Disable close button if tab size is too small Make tab close button respect tab's container Nov 9, 2016
@cezaraugusto
Copy link
Contributor Author

PR updated

@bbondy
Copy link
Member

bbondy commented Nov 15, 2016

Seems like an improvement but it doesn't get rid of the (x) and it makes the favicons get cut off when smaller. @bradleyrichter is this the way you want it?
screenshot 2016-11-15 17 09 55

@bbondy bbondy modified the milestones: 0.12.11, 0.12.10 release Nov 15, 2016
@bradleyrichter
Copy link
Contributor

@bbondy no, they should actually end up looking like fat pinned tabs, and eventually the same, but of course in the main tab area.

Fix #5431

Auditors: @bbondy, @bsclifton

Note:

Now at window's width < 770px (where our first text trim happens), close button is now centered filling 100% of tab's size.

Test Plan:

* Reduce window's width checking for close button on tabs
* Close button should not overflow tab container on narrow views, avoiding UI mismatch
* At < 770px close button should fill tab's container and be centered
@cezaraugusto
Copy link
Contributor Author

PR updated

We now have close button centered filling tab's width on small (< 770px) screens:

close_btn

@bradleyrichter
Copy link
Contributor

Much better…

Is it possible to have the close button appear on the right?

On Nov 21, 2016, at 6:21 AM, Cezar Augusto [email protected] wrote:

PR updated

We now have close button centered filling tab's width on small (< 770px) screens:

https://cloud.githubusercontent.com/assets/4672033/20485946/ee8a53de-afe4-11e6-8428-0cde3d1bb4b8.png

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #5480 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AM4jqudbAQO-qYWLA_puvUnvqEwr3tJrks5rAajUgaJpZM4Kr7Av.

@cezaraugusto
Copy link
Contributor Author

@bradleyrichter you mean something like this?

new_tab_close

If not, could you do a quick draft about how do you want it?

@bbondy bbondy modified the milestones: 0.13.1, 0.13.0 Nov 22, 2016
@cezaraugusto
Copy link
Contributor Author

Closing in favor of #5845

@bbondy bbondy removed this from the 0.13.1 milestone Nov 29, 2016
@cezaraugusto cezaraugusto deleted the feature/tabsbar/5431 branch July 25, 2017 07:28
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
5 participants