Skip to content

Conversation

@crisbeto
Copy link
Member

@crisbeto crisbeto commented Sep 3, 2017

Fixes the table row content not being centered vertically in IE due to a flex bug.

Fixes #6813.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 3, 2017
@andrewseguin andrewseguin added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Sep 7, 2017
@andrewseguin
Copy link
Contributor

LGTM

Was this a bug that was around before beta 10? I don't remember seeing it in IE before the release.

@crisbeto
Copy link
Member Author

crisbeto commented Sep 7, 2017

I'm not sure, I don't think I've seen it in earlier betas.

@mmalerba
Copy link
Contributor

@crisbeto some screenshot tests seem to show that this is increasing the row height, is that expected?

@mmalerba mmalerba added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Sep 12, 2017
@mmalerba
Copy link
Contributor

caretaker note: some screenshot tests in g3 show larger row height as a result of the PR, needs investigation

@crisbeto crisbeto added the target: patch This PR is targeted for the next patch release label Dec 6, 2017
@mmalerba
Copy link
Contributor

mmalerba commented Dec 7, 2017

Is there a way to do this so that it won't require people who are overriding the min-height on .mat-row to also have to override it on .mat-row::after?

If not i think we have to consider this a breaking change, a lot of people seem to be overriding this min-height value.

(removing release: patch, re-add if we find a non-breaking solution, otherwise we can add release: major)

@mmalerba mmalerba removed the target: patch This PR is targeted for the next patch release label Dec 7, 2017
@crisbeto crisbeto force-pushed the 6813/row-alignment-ie branch from f1cd91c to 587bc00 Compare December 8, 2017 18:24
@crisbeto
Copy link
Member Author

crisbeto commented Dec 8, 2017

Switched the min-height to inherit which still works and makes it more convenient to override. Setting back the patch label @mmalerba.

@crisbeto crisbeto added the target: patch This PR is targeted for the next patch release label Dec 8, 2017
@andrewseguin andrewseguin added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Dec 13, 2017
@oktav777
Copy link

Isn't this hacky? There is a better workaround. Just add

.mat-table {
    display: flex;
    flex-direction: column;
}

@andrewseguin
Copy link
Contributor

Looks like we're good on internal tests now so this can start getting in. Quick note though, can you add a link to the IE issue in the code? https://connect.microsoft.com/IE/feedback/details/802625

@oktav777 That works but we try to avoid setting display properties on the outermost component element containers

Fixes the table row content not being centered vertically in IE due to a flex bug.

Fixes angular#6813.
@crisbeto crisbeto force-pushed the 6813/row-alignment-ie branch from 587bc00 to 2e83066 Compare January 10, 2018 18:14
@crisbeto
Copy link
Member Author

Added that comment @andrewseguin.

@crisbeto crisbeto removed the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jan 10, 2018
@andrewseguin andrewseguin merged commit 1b79e92 into angular:master Jan 12, 2018
andrewseguin pushed a commit to andrewseguin/components that referenced this pull request Jan 12, 2018
Fixes the table row content not being centered vertically in IE due to a flex bug.

Fixes angular#6813.
andrewseguin pushed a commit that referenced this pull request Jan 17, 2018
Fixes the table row content not being centered vertically in IE due to a flex bug.

Fixes #6813.
@gizmodus
Copy link

I am having the same issue again in IE. The problem seems to be lying here (table.scss):

mat-cell, mat-header-cell, mat-footer-cell {
  flex: 1;
  display: flex;
  align-items: center;
  overflow: hidden;
  word-wrap: break-word;
  min-height: inherit;
}

If min-height: inherit; is removed, the cells are displayed correctly.

@b-mi
Copy link

b-mi commented Sep 19, 2018

I solved this problem with this style for IE. Work texts with badges.

@media all and (-ms-high-contrast: none), (-ms-high-contrast: active) {
  /* IE10+ CSS styles go here */
  .mat-header-row, .mat-row {
    -ms-flex-align: stretch;
  }
}

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Table] IE rows are not centered vertically

7 participants