Skip to content

fix(table): row content not centered in IE #6820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 12, 2018

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