Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

fix(tabs): min-width not respected on IE11 #11432

Merged
merged 1 commit into from
Nov 15, 2018
Merged

fix(tabs): min-width not respected on IE11 #11432

merged 1 commit into from
Nov 15, 2018

Conversation

Splaktar
Copy link
Member

@Splaktar Splaktar commented Aug 30, 2018

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

  • Tab items have right and left padding of 24px which is 12px more on each side than specified by the MD spec.
  • Tabs on IE11 are truncated by pagination due to a calculated width of 0px and the only width being the 48px of padding.
  • There are 2 functions in the code that are not used anymore.
  • Many of the tab styles use translate3d which causes performance issues and bugs on IE11. We don't actually translate anything in the z dimension.

Issue Number:
Fixes #10406

What is the new behavior?

  • tab-items should have 12px right/left padding instead of 24px
    • this is to align with the spec and more of the label visible on mobile
  • remove unused calcPagingWidth()
  • convert all translate3d to translate for better IE11 support

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

The breaking changes to make min-width align with the MD spec have been pulled out of this PR and put into PR #11438 for 1.2.0.

Please note that this is fixing a regression from a breaking change that was in 1.1.2.

Before (IE on bottom, Chrome on top):
chrome-and-ie-before-fix

After in IE
tabs-fixed-ie11-without-min-width-fix
tabs-pagination-ie11-fixed-without-min-width-fix

After in Chrome
tabs-pagination-chrome-without-min-width-fix

And here is IE11 rendering with a small width (sub 600px)
tabs-pagination-ie11-xs-fixed
tabs-fixed-ie11-xs

@Splaktar Splaktar added type: bug browser: IE This issue is specific to Internet Explorer pr: merge ready This PR is ready for a caretaker to review severity: regression This issue is related to a regression P2: required Issues that must be fixed. labels Aug 30, 2018
@Splaktar Splaktar added this to the 1.1.11 milestone Aug 30, 2018
@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Aug 30, 2018
@Splaktar Splaktar requested a review from jelbourn August 30, 2018 03:20
@Splaktar
Copy link
Member Author

@jelbourn please review and let me know if you think that increasing the min-width to 160px on gt-xs devices will cause too many problems in Google.

This change aligns with the spec and with Angular Material (other than it using 24px padding on right/left of tab items). Note that it looks like the 2018 MD spec actually suggests 16px padding, so that may be the right value for Angular Material.

@Splaktar
Copy link
Member Author

Splaktar commented Sep 4, 2018

The min-width change on gt-xs devices causes 100+ tests to fail in g3. So I'll need to pull that part of this PR into a separate PR for 1.2.0.

@Splaktar Splaktar added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs and removed pr: merge ready This PR is ready for a caretaker to review labels Sep 4, 2018
@Splaktar Splaktar assigned Splaktar and unassigned josephperrott Sep 4, 2018
@Splaktar Splaktar force-pushed the fixTabsIE branch 2 times, most recently from 7807da7 to 1a4eede Compare September 10, 2018 21:19
@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs pr: presubmit-failures labels Sep 10, 2018
Splaktar added a commit that referenced this pull request Sep 10, 2018
previous commits removed use of getMinTabWidth but never updated CSS
- this applies the proper min-width CSS to match the spec
- which is 72px on xs and 160px on everything else

Relates to #10406. Relates to #11432.
@Splaktar
Copy link
Member Author

Removed fix for min-width to align with the MD spec. Put those changes into PR #11438 for 1.2.0.

Updated OP screenshots to reflect these changes.

@Splaktar
Copy link
Member Author

This latest iteration "causes a slight layout change, which seems pretty minimal, but is screenshot test breaking." It looks like it will need approval from about 5 apps to get in.

After about 5 iterations now, I think this is the least disruptive we can get on this regression fix. Considering it breaks IE11 completely, I think that we need to get this in.

@Splaktar Splaktar added the Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. label Oct 20, 2018
@Splaktar Splaktar assigned mmalerba and jelbourn and unassigned josephperrott and mmalerba Oct 23, 2018
@Splaktar Splaktar assigned andrewseguin and unassigned jelbourn Nov 11, 2018
@andrewseguin andrewseguin merged commit 2b2f441 into master Nov 15, 2018
@Splaktar Splaktar removed Blocked Progress on this issue is blocked. Primarily used for PRs that are blocked by presubmit feedback. pr: presubmit-failures labels Nov 15, 2018
@Splaktar Splaktar deleted the fixTabsIE branch November 15, 2018 23:22
Splaktar added a commit that referenced this pull request Dec 26, 2018
only apply the width = 999999px work around when pagination is enabled
only one of center tabs or pagination can be activate at a time

Fixes #11566. Relates to #11432.
mmalerba pushed a commit that referenced this pull request Jan 3, 2019
only apply the width = 999999px work around when pagination is enabled
only one of center tabs or pagination can be activate at a time

Fixes #11566. Relates to #11432.
@Splaktar Splaktar changed the title fix(tabs): min-width not respected on IE11. fix right/left padding fix(tabs): min-width not respected on IE11 Apr 19, 2019
Splaktar added a commit that referenced this pull request Jun 29, 2020
previous commits removed use of getMinTabWidth but never updated CSS
- this applies the proper min-width CSS to match the spec
- which is 72px on xs and 160px on everything else

Relates to #10406. Relates to #11432.
Splaktar added a commit that referenced this pull request Jun 29, 2020
- previous commits removed use of `getMinTabWidth` but never updated the related CSS
  - this applies the proper `min-width` CSS to match the spec
  - which is `72px` on `xs` and `160px` on everything else
- this also changes the left and right padding from `24px` to `12px` to align with the spec

Relates to #10406. Relates to #11432.

BREAKING CHANGE: Tab items now have a `min-width` and `padding` which matches the Material Design specification. For width, this is `72px` on `xs` screens and `160px` on all other screens. For left and right `padding`, this is now `12px` instead of `24px`. If your app needs to have tabs which are smaller than the spec, you will need to override `md-tab-item`'s `min-width` and `md-tab`'s `padding` styles.
Splaktar added a commit that referenced this pull request Jun 30, 2020
- previous commits removed use of `getMinTabWidth` but never updated the related CSS
  - this applies the proper `min-width` CSS to match the spec
  - which is `72px` on `xs` and `160px` on everything else
- this also changes the left and right padding from `24px` to `12px` to align with the spec

Relates to #10406. Relates to #11432.

BREAKING CHANGE: Tab items now have a `min-width` and `padding` which matches the Material Design specification. For width, this is `72px` on `xs` screens and `160px` on all other screens. For left and right `padding`, this is now `12px` instead of `24px`. If your app needs to have tabs which are smaller than the spec, you will need to override `md-tab-item`'s `min-width` and `md-tab`'s `padding` styles.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
browser: IE This issue is specific to Internet Explorer cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P2: required Issues that must be fixed. pr: merge ready This PR is ready for a caretaker to review severity: regression This issue is related to a regression type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

md-tabs: text turns to dots on small screen on IE11
6 participants