Skip to content

Commit

Permalink
fix(list): remove secondary container flex filler.
Browse files Browse the repository at this point in the history
* Currently the secondary container was pushed to the end of the list-item by using a flex element with auto.
  This is causing issues with custom content of developers, because the flex filler has a high priority and reserves space.

* This can be fixed, by using a margin auto, which has a lower priority and isn't reserving any space.
   This allows the user to style the content himself and we won't interfere.

* The height of 100% for the secondary item container was depending on its parent DOM structure. So in some weird cases, the secondary item container was reserving 100% height of the screen. This property even became unnecessary, because now the secondary items are expecting its size correctly and will fill the remaining space by the auto margin.

Fixes angular#8094. Fixes angular#7976.
  • Loading branch information
devversion committed May 30, 2016
1 parent c9158c8 commit f953a68
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 18 deletions.
4 changes: 0 additions & 4 deletions src/components/list/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,6 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
wrapSecondaryItem(secondaryItem, secondaryItemsWrapper);
});

// Since the secondary item container is static we need to fill the remaing space.
var spaceFiller = angular.element('<div class="flex"></div>');
itemContainer.append(spaceFiller);

itemContainer.append(secondaryItemsWrapper);
}

Expand Down
13 changes: 11 additions & 2 deletions src/components/list/list.scss
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,18 @@ md-list-item {
display: flex;
align-items: center;

height: 100%;
// The secondary container is now static and needs to be aligned at the end of its parent.
// - Using an absolute position will cause much issues with the overflow.
// - Using a flex-filler, which pushes the container to the end of the parent is working
// but breaks the users list-item layout.
// Using margin auto to move them to the end of the list item is more elegant, because it has
// a lower priority than the flex filler and isn't introducing overflow issues again.
// The margin on the top is also important to align multiple secondary items vertical correctly.
margin: auto;

@include rtl(margin-right, 0, auto);
@include rtl(margin-left, auto, 0);

.md-button, .md-icon-button {
&:last-of-type {
// Reset 6px margin for the button.
Expand Down Expand Up @@ -346,7 +355,7 @@ md-list-item {
overflow: hidden;

&.md-offset {
@include rtl-prop(margin-left, margin-right, $list-item-primary-width);
@include rtl-prop(margin-left, margin-right, $list-item-primary-width);
}

h3 {
Expand Down
24 changes: 12 additions & 12 deletions src/components/list/list.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ describe('mdListItem directive', function() {
var buttonWrap = listItem.children().eq(0);
expect(listItem).toHaveClass('_md-button-wrap');

// The button wrap should contain the button executor, the inner content, flex filler and the
// The button wrap should contain the button executor, the inner content and the
// secondary item container as children.
expect(buttonWrap.children().length).toBe(4);
expect(buttonWrap.children().length).toBe(3);

var buttonExecutor = buttonWrap.children()[0];

Expand All @@ -164,9 +164,9 @@ describe('mdListItem directive', function() {
var buttonWrap = listItem.children().eq(0);
expect(listItem).toHaveClass('_md-button-wrap');

// The button wrap should contain the button executor, the inner content, flex filler and the
// The button wrap should contain the button executor, the inner content and the
// secondary item container as children.
expect(buttonWrap.children().length).toBe(4);
expect(buttonWrap.children().length).toBe(3);

var buttonExecutor = buttonWrap.children()[0];

Expand All @@ -189,9 +189,9 @@ describe('mdListItem directive', function() {
var buttonWrap = listItem.children().eq(0);
expect(listItem).toHaveClass('_md-button-wrap');

// The button wrap should contain the button executor, the inner content, flex filler and the
// The button wrap should contain the button executor, the inner content and the
// secondary item container as children.
expect(buttonWrap.children().length).toBe(4);
expect(buttonWrap.children().length).toBe(3);

var buttonExecutor = buttonWrap.children()[0];

Expand Down Expand Up @@ -230,11 +230,11 @@ describe('mdListItem directive', function() {

expect(listItem).toHaveClass('_md-button-wrap');

// It should contain three elements, the button overlay, inner content, flex filler
// It should contain three elements, the button overlay, inner content
// and the secondary container.
expect(firstChild.children().length).toBe(4);
expect(firstChild.children().length).toBe(3);

var secondaryContainer = firstChild.children().eq(3);
var secondaryContainer = firstChild.children().eq(2);
expect(secondaryContainer).toHaveClass('_md-secondary-container');

// The secondary container should contain the md-icon,
Expand All @@ -256,11 +256,11 @@ describe('mdListItem directive', function() {

expect(listItem).toHaveClass('_md-button-wrap');

// It should contain three elements, the button overlay, inner content, flex filler
// It should contain three elements, the button overlay, inner content,
// and the secondary container.
expect(firstChild.children().length).toBe(4);
expect(firstChild.children().length).toBe(3);

var secondaryContainer = firstChild.children().eq(3);
var secondaryContainer = firstChild.children().eq(2);
expect(secondaryContainer).toHaveClass('_md-secondary-container');

// The secondary container should hold the two secondary items.
Expand Down

0 comments on commit f953a68

Please sign in to comment.