fix(flexbox): scan flex-direction in css stylesheet #365

ThomasBurleson · 2017-08-06T12:35:51Z

fxFlex works best if the parent container has flexbox styles assigned (display, flex-direction, etc.). If not assigned (inline or via stylesheet), then these styles are auto-injected inline on the DOM parent element.

When scanning parent DOM element for flexbox flow direction, first scan the in element's inline style and then scan the computed stylesheet for the flex-direction value.

It is assumed that if the parent element has a flex-direction style property set, then the other required properties (display, border-box, etc.) have also been assigned.

Fixes #272, #364.

ThomasBurleson · 2017-08-06T12:39:26Z

@crisbeto - can you review ? Thx.

crisbeto

LGTM, a few nits.

crisbeto · 2017-08-06T13:13:19Z

src/lib/flexbox/api/base.ts

@@ -146,7 +152,7 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
    try {
      if (element) {
        let immediateValue = getDom().getStyle(element, styleName);
-        value = immediateValue || getDom().getComputedStyle(element).display;
+        value = immediateValue || getDom().getComputedStyle(element)[styleName];


Instead of getDom().getComputedStyle(element)[styleName] you can use getDom().getComputedStyle(element).getPropertyValue(styleName).

IYO, which is preferred/better ?

I believe getPropertyValue guarantees that the style will be recalculated.

Good to know. Thx.

As I know, in iOS 9 the getPropertyValue returns null instead of empty string as in browsers on other OS. Not sure how much people still use the iOS 9 so my comment is just for info.

crisbeto · 2017-08-06T13:14:17Z

src/lib/flexbox/api/flex.ts

+  /**
+   * Constructor
+   *
+   * Note: Explicitly @SkipSelf on LayoutDirective and LayoutWrapDirective because we are looking


This note might be better as a regular comment, otherwise it'll show up in any generated docs and typings.

Good point. Thx.

crisbeto · 2017-08-06T13:14:56Z

src/lib/flexbox/api/flex.spec.ts

@@ -118,6 +118,26 @@ describe('flex directive', () => {
      expect(element).not.toHaveCssStyle({'min-width': '30px'});
    });

+    it('should CSS stylesheet and not inject flex-direction on parent', () => {
+      componentWithTemplate(`
+        <style type="text/css">


The text/css shouldn't be necessary.

k. will remove.

crisbeto · 2017-08-06T13:16:09Z

src/lib/flexbox/api/base.ts

@@ -123,6 +123,12 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
    return value ? value.trim() : ((element.nodeType === 1) ? 'block' : 'inline-block');


Not something that was added in this PR, but instead of hardcoding the 1, you can use the built-in browser constant: element.nodeType === Node.ELEMENT_NODE.

ThomasBurleson · 2017-08-06T23:02:52Z

@crisbeto using Node.ELEMENT_NODE causes problems with angular universal compiles.

`fxFlex` works best if the parent container has flexbox styles assigned (display, flex-direction, etc.). If not assigned (inline or via stylesheet), then these styles are auto-injected inline on the DOM parent element. When scanning parent DOM element for flexbox flow direction, first scan the in element's inline style and then scan the computed stylesheet for the `flex-direction` value. > It is assumed that if the parent element has a `flex-direction` style property set, then the other required properties (display, border-box, etc.) have also been assigned. Fixes #272, #364.

matheusdavidson · 2017-11-14T03:47:37Z

Sorry for saying this but i think you guys are trying to apply styles where you don't need to. This is causing many issues and non expected behaviors.

I'm saying this because i had some problems already with things like using fxFlex.gt-xs and flex layout adding styles even when breakpoint is under xs(talked about this in another issue #296).

@ThomasBurleson, you said: "fxFlex works best if the parent container has flexbox styles assigned.."

I ask you: Wouldn't it be better to let the user choose if he wants a flexbox parent?

Really, i think that worse then having a fxFlex without flexbox parent(this obvious) is to have no idea why my parent element got flexbox in the first place, since i was already applying it in my css file and flex layout just override the whole thing.

angular-automatic-lock-bot · 2019-09-06T01:02:05Z

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.}

googlebot added the cla: yes label Aug 6, 2017

ThomasBurleson added this to the v2.0.0-beta.9 milestone Aug 6, 2017

crisbeto approved these changes Aug 6, 2017

View reviewed changes

ThomasBurleson force-pushed the thomas/issue-364 branch from dde889d to 07f9a63 Compare August 6, 2017 13:48

ThomasBurleson added pr: lgtm This PR has been approved by the reviewer pr: needs presubmit labels Aug 6, 2017

ThomasBurleson force-pushed the thomas/issue-364 branch from 07f9a63 to 55a7636 Compare August 6, 2017 23:01

ThomasBurleson force-pushed the thomas/issue-364 branch 3 times, most recently from 90f4238 to 16f8656 Compare August 7, 2017 13:52

ThomasBurleson force-pushed the thomas/issue-364 branch from 16f8656 to 4d16c03 Compare August 7, 2017 13:54

mmalerba merged commit 635c4f5 into master Aug 7, 2017

ThomasBurleson deleted the thomas/issue-364 branch August 7, 2017 22:35

rosslavery mentioned this pull request Nov 13, 2017

flex-direction: row; box-sizing: border-box; display: flex; added automatically #496

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(flexbox): scan flex-direction in css stylesheet #365

fix(flexbox): scan flex-direction in css stylesheet #365

ThomasBurleson commented Aug 6, 2017 •

edited

Loading

ThomasBurleson commented Aug 6, 2017 •

edited

Loading

crisbeto left a comment

crisbeto Aug 6, 2017

ThomasBurleson Aug 6, 2017

crisbeto Aug 6, 2017 •

edited

Loading

ThomasBurleson Aug 6, 2017

Matrosskin Feb 26, 2019

crisbeto Aug 6, 2017

ThomasBurleson Aug 6, 2017

crisbeto Aug 6, 2017

ThomasBurleson Aug 6, 2017

crisbeto Aug 6, 2017

ThomasBurleson Aug 6, 2017

ThomasBurleson commented Aug 6, 2017

matheusdavidson commented Nov 14, 2017

angular-automatic-lock-bot bot commented Sep 6, 2019

		@@ -123,6 +123,12 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
		return value ? value.trim() : ((element.nodeType === 1) ? 'block' : 'inline-block');

fix(flexbox): scan flex-direction in css stylesheet #365

fix(flexbox): scan flex-direction in css stylesheet #365

Conversation

ThomasBurleson commented Aug 6, 2017 • edited Loading

ThomasBurleson commented Aug 6, 2017 • edited Loading

crisbeto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crisbeto Aug 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomasBurleson commented Aug 6, 2017

matheusdavidson commented Nov 14, 2017

angular-automatic-lock-bot bot commented Sep 6, 2019

ThomasBurleson commented Aug 6, 2017 •

edited

Loading

ThomasBurleson commented Aug 6, 2017 •

edited

Loading

crisbeto Aug 6, 2017 •

edited

Loading