Skip to content

Hide tab fade until text overflows with sizing-shrink and close-button-left/off (fix #45728)#45815

Merged
bpasero merged 2 commits intomicrosoft:masterfrom
Dari-K:tab-fade-45728
Mar 22, 2018
Merged

Hide tab fade until text overflows with sizing-shrink and close-button-left/off (fix #45728)#45815
bpasero merged 2 commits intomicrosoft:masterfrom
Dari-K:tab-fade-45728

Conversation

@Dari-K
Copy link
Contributor

@Dari-K Dari-K commented Mar 15, 2018

Moved the padding-right into the label text, which should hide the tab fade while there aren't enough tabs to fill the tab bar (or until a tab gets the .dirty class) #45728

@bpasero bpasero self-assigned this Mar 15, 2018
Copy link
Member

Choose a reason for hiding this comment

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

@Dari-K is that an oversight that you removed this rule for good?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bpasero Well the padding-right was moved to another selector so this rule was empty, should I have left the selector with an empty block?

Copy link
Member

@bpasero bpasero Mar 16, 2018

Choose a reason for hiding this comment

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

@Dari-K but I am no longer seeing the padding to the right when tab buttons are off? Please check both for sizing fit and shrink

Since this padding is for the ::after element, would it make sense to add the padding to that element if possible?

Copy link
Contributor Author

@Dari-K Dari-K Mar 16, 2018

Choose a reason for hiding this comment

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

@bpasero yeah, you're totally right, and that would make sense, will fix later.
Edit: actually I think putting the padding on ::after meant that the fade was visible even when the text wasn't overflowing, will check later.

Copy link
Member

Choose a reason for hiding this comment

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

@Dari-K ok thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bpasero so I fixed the problem with the missing padding, but realised that this solution still may not be good enough. The way it works is moving the padding-right to the label text, which collapses, so when the text isn't overflowing the fade is over the empty padding, and when the padding collapses the fade will be over the text. However the padding-right previously didn't collapse when the tab shrank, so this is a change in behaviour.

Do we have a way to tell when the label is overflowing? Or can you think of another way to hide the fade when we shouldn't be showing it?

Copy link
Member

Choose a reason for hiding this comment

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

@Dari-K no we do not, but maybe we could just do this: it seems to work well when tab-close button is on the right and the reason for that is that we reserve some space on the right hand side of the tab for the close button even if the close button is not visible:

image

What if we would reserve some space to the right also when tab-close button is left or off in case tab sizing is shrink? I assume we would not need much more than 5px

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bpasero I added a pseudo element to reserve 5px on right for sizing shrink with close button left or off like you said. Seems to work well. 👍

@bpasero bpasero added this to the March 2018 milestone Mar 15, 2018
@bpasero bpasero merged commit 2d686ce into microsoft:master Mar 22, 2018
@bpasero
Copy link
Member

bpasero commented Mar 22, 2018

@Dari-K good stuff 👍

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants