Skip to content
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

MdGridList cannot switch between ratio and fixed row height #4047

Closed
arlowhite opened this issue Apr 12, 2017 · 1 comment · Fixed by #6660
Closed

MdGridList cannot switch between ratio and fixed row height #4047

arlowhite opened this issue Apr 12, 2017 · 1 comment · Fixed by #6660
Assignees
Labels
help wanted The team would appreciate a PR from the community to address this issue P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@arlowhite
Copy link

Bug:

Switching from ratio to fixed height causes tiles to have larger heights than they should.

This is because RatioTileStyler uses paddingTop and FixedTileStyler uses height to set tile heights. But when switching between them, the old styles are not cleared.

What is the expected behavior?

Users should be able change rowHeight between ratio, fixed, and fit dynamically.

What is the current behavior?

Switching between fixed and ratio rowHeight modes results in tiles that are larger than they should be due to left-over styles.
Have not tested 'fit' mode.

What are the steps to reproduce?

I reproduced this with unit tests in grid-list.spec.ts
https://github.com/arlowhite/material2/tree/grid-rowheight-bug

What is the use-case or motivation for changing an existing behavior?

Users can hit this bug easily, even if they aim to stick with fixed mode.
If their Component loads with rowHeight undefined, grid will default to "1:1"
Then when they set a number to rowHeight, this bug will occur.

Which versions of Angular, Material, OS, browsers are affected?

Affects 2.0.0-beta.3
material2 master 9d719c5

Angular 4.0.1

Is there anything else we should know?

I will submit a PR with the unit test.

@jelbourn jelbourn added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent effort1: easy (hours) help wanted The team would appreciate a PR from the community to address this issue labels Aug 17, 2017
@crisbeto crisbeto self-assigned this Aug 27, 2017
crisbeto added a commit to crisbeto/material2 that referenced this issue Aug 27, 2017
…ing mode

Fixes the old styles not being cleared when switching between styling modes in the grid list (e.g. going for proportions to a fixed height).

Fixes angular#4047.
jelbourn pushed a commit that referenced this issue Aug 30, 2017
…ing mode

Fixes the old styles not being cleared when switching between styling modes in the grid list (e.g. going for proportions to a fixed height).

Fixes #4047.
crisbeto added a commit to crisbeto/material2 that referenced this issue Sep 1, 2017
…ing mode

Fixes the old styles not being cleared when switching between styling modes in the grid list (e.g. going for proportions to a fixed height).

Fixes angular#4047.
tinayuangao pushed a commit that referenced this issue Sep 6, 2017
…ing mode (#6660)

Fixes the old styles not being cleared when switching between styling modes in the grid list (e.g. going for proportions to a fixed height).

Fixes #4047.
@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 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted The team would appreciate a PR from the community to address this issue P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants