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

fix(layout): Give Non-numeric flex values priority when media-dependent #11469

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bersLucas
Copy link
Contributor

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?

Issue Number:
#11466

.layout-column > .flex-##, .layout-row > .flex-## Exists for numeric flex values, but do not for non-numeric flex values, (grow, auto, ect). This means that using a numeric flex value with a non-numeric flex value with a gt/sm/xs prefix will only render the numeric one, even if the media rules should prefer the non-numeric one.

<div layout="row">
  <div layout="column" flex="100" flex-gt-sm="auto">
    <!-- will always be [flex="100"], even when gt-sm is true -->
  </div>
</div>

What is the new behavior?

Add more specific rules to the non-numeric mixins,
.layout-row/column > .#{$flexName}-nogrow/initial/auto/noshrink/grow/none.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Oct 2, 2018
@codymikol
Copy link
Contributor

:partyparrot:

@Splaktar Splaktar self-requested a review October 3, 2018 21:30
@Splaktar Splaktar self-assigned this Oct 3, 2018
@Splaktar Splaktar added ui: layout P4: minor Minor issues. May not be fixed without community contributions. labels Oct 3, 2018
@Splaktar Splaktar added this to the 1.1.12 milestone Oct 3, 2018
@Splaktar
Copy link
Member

Splaktar commented Oct 3, 2018

As with any changes to layout, we need to evaluate how the change affects the bundle size of the generated CSS.

Can you please post the size numbers of before/after this PR for the angular-material.css and angular-material.min.css files?

I'd love to say that we need some tests for this, however, I realize that our test setup for layouts is somewhere close to inadequate or non-existent. I'll try to do some manual testing on this soon.

@Splaktar Splaktar added the needs: manual testing This issue or PR needs to have some manual testing and verification done label Oct 3, 2018
@bersLucas
Copy link
Contributor Author

repository angular-material.css angular-material.min.css
angular:master 428kb 322kb
bersLucas:master 448kb (+20kb) 339kb (+17kb)

@Splaktar Splaktar added needs: team discussion This issue requires a decision from the team before moving forward. type: enhancement labels Nov 9, 2018
@Splaktar Splaktar modified the milestones: 1.1.12, 1.1.13 Jan 3, 2019
@Splaktar Splaktar modified the milestones: 1.1.13, 1.1.14 Feb 10, 2019
@Splaktar Splaktar modified the milestones: 1.1.14, 1.1.15 Mar 14, 2019
@Splaktar Splaktar modified the milestones: 1.1.15, 1.1.16, 1.1.18, 1.1.19 Mar 29, 2019
@Splaktar Splaktar modified the milestones: 1.1.19, 1.1.20 May 30, 2019
@Splaktar Splaktar modified the milestones: 1.1.20, 1.1.21 Aug 15, 2019
@Splaktar Splaktar modified the milestones: 1.1.21, 1.1.22 Sep 30, 2019
@Splaktar Splaktar removed this from the 1.1.22 milestone Oct 22, 2019
@Splaktar Splaktar added this to the 1.1.23 milestone Oct 22, 2019
@Splaktar Splaktar modified the milestones: 1.1.23, 1.2.0 Jun 8, 2020
@Splaktar Splaktar modified the milestones: 1.2.0, 1.3.0 Jul 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ needs: manual testing This issue or PR needs to have some manual testing and verification done needs: team discussion This issue requires a decision from the team before moving forward. P4: minor Minor issues. May not be fixed without community contributions. type: enhancement ui: layout
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants