fix(lib): support API directive queries #290

ThomasBurleson · 2017-05-19T00:05:17Z

support DI of flexbox/api Directive classes
implement Directive method activatedValue getter/setter
- to imperatively read current activated input value
- to write API values with immediate style updates
implement demo using splitters (ngxSplit) with fxLayout and fxFlex layouts

Based on issues and ideas by @amcdnl + ngx-u

Fixes #266.

ThomasBurleson · 2017-05-19T00:19:49Z

Snapshot of Splitter Demo

issue.266.demo.ts

<div fxLayout="row" style="height:500px" ngxSplit="row">
  <div fxFlex="30%" ngxSplitArea class="c1r1" > </div>

  <div class="handle handle-row" ngxSplitHandle>
      <i class="material-icons">&#xE25D;</i>
  </div>
  
  <div fxFlex="70%" ngxSplitArea>
    <div fxLayout="column" fxFlexFill ngxSplit="column">
      <div fxFlex="50%" ngxSplitArea class="c2r1_body" > </div>
      
      <div class="handle handle-column" ngxSplitHandle>
          <i class="material-icons">&#xE25D;</i>
      </div>
      
      <div fxFlex="50%" ngxSplitArea class="c2r2" > </div>
    </div>
  </div>
</div>

split.directive.ts

  onDrag({x, y}): void {
    const dragAmount = (this.direction === 'row') ? x : y;

    this.areas.forEach((area, i) => {
      const flex = (area.flex as FlexDirective);
      const delta = (i === 0) ? dragAmount : -dragAmount;
      const currentValue = flex.activatedValue;        

      // Update Flex-Layout value to build/inject new flexbox CSS
      flex.activatedValue = this.calculateSize(currentValue, delta);
    });
  }

@amcdnl - would you mind reviewing ?

ThomasBurleson · 2017-05-19T12:02:15Z

@kara - please review.

amcdnl · 2017-05-19T12:21:26Z

@ThomasBurleson - This looks awesome!

ThomasBurleson · 2017-05-22T22:35:12Z

@jelbourn - please presubmit.

jelbourn · 2017-05-22T22:54:33Z

@mmalerba should take a look

mmalerba · 2017-05-23T14:45:17Z

src/lib/flexbox/api/base.ts

+
+    let activatedKey = this._mqActivation.activatedInputKey;
+    this._inputMap[activatedKey] = value;
+    this._updateStyle(value);


I think this logic can be simplified:

if (this._mqActivation) { this._inputMap[this._mqActivation.activatedInputKey] = value; } this._updateStyle(value);

mmalerba · 2017-05-23T14:46:47Z

src/lib/flexbox/api/base.ts

@@ -65,10 +91,17 @@ export abstract class BaseFxDirective implements OnDestroy {
  // *********************************************

  /**
+   * Abstract method...


Needs documentation, also should it throw an error that it needs to be implemented?

Thanks for drawing my attention to this. This was only a partial (in fact poor) solution.
I found another more robust approach.

mmalerba · 2017-05-23T14:47:35Z

src/lib/flexbox/api/base.ts

-          value = value || 'row';
-          this._applyStyleToElements(buildLayoutCSS(value), [target]);
-        }
+    let value = "";


nit: single quotes

mmalerba · 2017-05-23T14:52:04Z

src/lib/flexbox/api/base.ts

+        return direction || styles[key];
+      }, null);
+
+      let immediateValue = findDirection(target['style']);


why this instead of target.style?

cannot recall the reasoning. Will fix.

mmalerba · 2017-05-23T14:53:07Z

src/lib/flexbox/api/base.ts

+      }, null);
+
+      let immediateValue = findDirection(target['style']);
+      value = immediateValue || findDirection(getComputedStyle(target as Element));


is there a reason that target isn't an Element in the first place?

this._elementRef.nativeElement is typed as any... so I cast here to document explicitly.

mmalerba · 2017-05-23T14:54:41Z

src/lib/flexbox/api/base.ts


-      return value ? value.trim() : "row";
+    return value ? value.trim() : "row";


nit: single quotes

mmalerba · 2017-05-23T15:05:26Z

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

@@ -126,15 +128,15 @@ describe('flex directive', () => {
      `);
      fixture.detectChanges();
      let element = queryFor(fixture, "[fxFlex]")[0].nativeElement;
-      let parent  = queryFor(fixture, ".test")[0].nativeElement;
+      let parent = queryFor(fixture, ".test")[0].nativeElement;


single quotes

mmalerba · 2017-05-23T15:05:37Z

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

@@ -149,11 +151,11 @@ describe('flex directive', () => {
      `);
      fixture.detectChanges();
      let element = queryFor(fixture, "[fxFlex]")[0].nativeElement;
-      let parent  = queryFor(fixture, ".parent")[0].nativeElement;
+      let parent = queryFor(fixture, ".parent")[0].nativeElement;


single quotes

ThomasBurleson · 2017-05-23T21:16:07Z

src/lib/flexbox/api/base.ts

@@ -65,10 +91,17 @@ export abstract class BaseFxDirective implements OnDestroy {
  // *********************************************

  /**
+   * Abstract method...


Accidentally deleted Miles comments: docs needed and throw exception if not overriden

ThomasBurleson · 2017-05-23T21:16:55Z

src/lib/flexbox/api/base.ts

+
+    let activatedKey = this._mqActivation.activatedInputKey;
+    this._inputMap[activatedKey] = value;
+    this._updateStyle(value);


Accidently deleted @mmalerba comments. Refactor ing

ThomasBurleson · 2017-05-23T21:17:32Z

src/lib/flexbox/api/base.ts

+        return direction || styles[key];
+      }, null);
+
+      let immediateValue = findDirection(target['style']);


cannot recall the reasoning. Will fix.

ThomasBurleson · 2017-05-23T21:20:30Z

src/lib/flexbox/api/base.ts

+      }, null);
+
+      let immediateValue = findDirection(target['style']);
+      value = immediateValue || findDirection(getComputedStyle(target as Element));


this._elementRef.nativeElement is typed as any... so I cast here to document explicitly.

ThomasBurleson · 2017-05-23T22:11:04Z

src/lib/flexbox/api/base.ts

@@ -65,10 +91,17 @@ export abstract class BaseFxDirective implements OnDestroy {
  // *********************************************

  /**
+   * Abstract method...


Thanks for drawing my attention to this. This was only a partial (in fact poor) solution.
I found another more robust approach.

ThomasBurleson · 2017-05-23T22:22:02Z

@mmalerba - updated with all your suggested changes. set activatedInput(value) now triggers ngOnChanges(); see https://github.com/angular/flex-layout/pull/290/files#diff-abac303936dc314fed0f18d5429ac42f

* support DI of flexbox/api Directive classes * implement Directive method `activatedValue` getter/setter * to imperatively read current activated input value * to write API values with immediate style updates * implement demo using splitters (ngxSplit) with fxLayout and fxFlex layouts

angular-automatic-lock-bot · 2019-09-05T22:03:54Z

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 May 19, 2017

ThomasBurleson force-pushed the thomas/issue-266 branch from ea7da67 to 6944d31 Compare May 19, 2017 00:17

ThomasBurleson force-pushed the thomas/issue-266 branch 3 times, most recently from 3aaf0e0 to b8a5852 Compare May 19, 2017 11:48

ThomasBurleson added the pr: needs review label May 19, 2017

ThomasBurleson requested a review from kara May 19, 2017 12:11

ThomasBurleson added tests pr: needs cleanup/tests in-progress and removed tests labels May 19, 2017

ThomasBurleson force-pushed the thomas/issue-266 branch 2 times, most recently from 7e05942 to b9be116 Compare May 19, 2017 23:12

ThomasBurleson added pr: needs presubmit and removed in-progress pr: needs cleanup/tests pr: needs review labels May 19, 2017

mmalerba reviewed May 23, 2017

View reviewed changes

ThomasBurleson commented May 23, 2017

View reviewed changes

ThomasBurleson force-pushed the thomas/issue-266 branch from b9be116 to db97d73 Compare May 23, 2017 22:19

ThomasBurleson force-pushed the thomas/issue-266 branch from db97d73 to 2ebb335 Compare May 23, 2017 22:33

mmalerba approved these changes May 24, 2017

View reviewed changes

mmalerba added the pr: lgtm This PR has been approved by the reviewer label May 24, 2017

tinayuangao merged commit f5558de into master May 24, 2017

ThomasBurleson mentioned this pull request Jul 16, 2017

Does it support layout resize by the user? #340

Closed

ThomasBurleson deleted the thomas/issue-266 branch August 7, 2017 22:36

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(lib): support API directive queries #290

fix(lib): support API directive queries #290

ThomasBurleson commented May 19, 2017 •

edited

Loading

ThomasBurleson commented May 19, 2017 •

edited

Loading

ThomasBurleson commented May 19, 2017

amcdnl commented May 19, 2017

ThomasBurleson commented May 22, 2017

jelbourn commented May 22, 2017

mmalerba May 23, 2017

mmalerba May 23, 2017

ThomasBurleson May 23, 2017

mmalerba May 23, 2017

mmalerba May 23, 2017

ThomasBurleson May 23, 2017

mmalerba May 23, 2017

ThomasBurleson May 23, 2017

mmalerba May 23, 2017

mmalerba May 23, 2017

mmalerba May 23, 2017

ThomasBurleson May 23, 2017

ThomasBurleson May 23, 2017

ThomasBurleson May 23, 2017

ThomasBurleson May 23, 2017

ThomasBurleson May 23, 2017

ThomasBurleson commented May 23, 2017

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


		return value ? value.trim() : "row";
		return value ? value.trim() : "row";

fix(lib): support API directive queries #290

fix(lib): support API directive queries #290

Conversation

ThomasBurleson commented May 19, 2017 • edited Loading

ThomasBurleson commented May 19, 2017 • edited Loading

Snapshot of Splitter Demo

issue.266.demo.ts

split.directive.ts

ThomasBurleson commented May 19, 2017

amcdnl commented May 19, 2017

ThomasBurleson commented May 22, 2017

jelbourn commented May 22, 2017

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

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 May 23, 2017

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

ThomasBurleson commented May 19, 2017 •

edited

Loading

ThomasBurleson commented May 19, 2017 •

edited

Loading