From f953a6858484b29e0aef15db14ab5d8a62b1373a Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 30 May 2016 19:49:57 +0200 Subject: [PATCH] fix(list): remove secondary container flex filler. * 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 #8094. Fixes #7976. --- src/components/list/list.js | 4 ---- src/components/list/list.scss | 13 +++++++++++-- src/components/list/list.spec.js | 24 ++++++++++++------------ 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/components/list/list.js b/src/components/list/list.js index e627f6a9d42..1de4fdc4978 100644 --- a/src/components/list/list.js +++ b/src/components/list/list.js @@ -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('
'); - itemContainer.append(spaceFiller); - itemContainer.append(secondaryItemsWrapper); } diff --git a/src/components/list/list.scss b/src/components/list/list.scss index 49138f04b7f..c29deca9848 100644 --- a/src/components/list/list.scss +++ b/src/components/list/list.scss @@ -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. @@ -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 { diff --git a/src/components/list/list.spec.js b/src/components/list/list.spec.js index 32df25231b9..27e476b5ced 100644 --- a/src/components/list/list.spec.js +++ b/src/components/list/list.spec.js @@ -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]; @@ -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]; @@ -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]; @@ -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, @@ -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.