fix(lib, media-query): support angular/universal #353

ThomasBurleson · 2017-07-29T20:02:55Z

Use getDom() for platform-server/universal fixes.
Now gets document object from platform-browser by DI instead of global.

Add universal application to packages

Fixes #187, #354. Closes #346.

Thx to @ardatan for PR #346.

ardatan · 2017-07-30T16:30:15Z

src/universal-app/app/splitter/split-handle.directive.ts

+  constructor(ref: ElementRef) {
+    const getMouseEventPosition = (event: MouseEvent) => ({x: event.movementX, y: event.movementY});
+
+    const mouseup$ = Observable.fromEvent(document, 'mouseup');


I think you have to inject 'DOCUMENT' from 'platform-browser'.

@ardatan - true. I think even better is to use ref.nativeElement.ownerDocument. Your thoughts ?

@ThomasBurleson I am not sure if platform-server has ownerDocument property implementation.

ThomasBurleson · 2017-07-30T17:27:02Z

@ardatan - BTW, even with injection or ownerDocument I am still getting gulp ci:prerender errors:

11:14:25] 'prerender' errored after 611 ms
[11:14:25] Error: ERROR TypeError: Invalid event target
    at Function.FromEventObservable.setupSubscription (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/rxjs/observable/FromEventObservable.js:113:19)
    at FromEventObservable._subscribe (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/rxjs/observable/FromEventObservable.js:135:29)
    at FromEventObservable.Observable._trySubscribe (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/rxjs/Observable.js:171:25)
    at FromEventObservable.Observable.subscribe (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/rxjs/Observable.js:159:65)
    at MapOperator.call (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/rxjs/operator/map.js:54:23)
    at Observable.subscribe (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/rxjs/Observable.js:156:22)
    at SwitchMapOperator.call (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/rxjs/operator/switchMap.js:67:23)
    at Observable.subscribe (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/rxjs/Observable.js:156:22)
    at SplitDirective.ngAfterContentInit (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/dist/packages/universal-app/app/splitter/split.directive.js:18:41)
    at callProviderLifecycles (/Users/thomasburleson/Documents/projects/Google/projects/angular-layouts/node_modules/@angular/core/bundles/core.umd.js:11206:18)

Any ideas ?

ardatan · 2017-07-30T17:56:05Z

in 'split-handle-directive.ts', I think it had better to take events into Observable by using HostListener. I changed like that, and ran ci:prerender. It was successful. You can try.
BTW, ngAfterViewInit looks better than ctor.

import {Directive, ElementRef, Output, Inject, EventEmitter, HostListener} from '@angular/core';
import {DOCUMENT} from '@angular/platform-browser';
import {Observable} from 'rxjs/Observable';

import 'rxjs/add/observable/fromEvent';
import 'rxjs/add/operator/takeUntil';
import 'rxjs/add/operator/switchMap';
import 'rxjs/add/operator/map';

@Directive({
  selector: '[ngxSplitHandle]',
  host: {
    class: 'ngx-split-handle',
    title: 'Drag to resize'
  }
})
export class SplitHandleDirective {

  @Output() drag: Observable<{ x: number, y: number }>;
  mousedown$ = new EventEmitter();

  constructor(private ref: ElementRef, @Inject(DOCUMENT) private document: any) { }
  ngAfterViewInit() {
    const getMouseEventPosition = (event: MouseEvent) => ({ x: event.movementX, y: event.movementY });

    const mouseup$ = Observable.fromEvent(this.document, 'mouseup');
    const mousemove$ = Observable.fromEvent(this.document, 'mousemove')
      .map(getMouseEventPosition);
    this.drag = this.mousedown$.switchMap(_ => mousemove$.takeUntil(mouseup$));

  }
  @HostListener('mousedown', ['$event']) onMouseDown(event) {
    this.mousedown$.next(event);
  }

}

ThomasBurleson · 2017-07-30T18:07:36Z

@ardatan - fixed it. Need to use isBrowser() utility method.

ardatan · 2017-07-30T18:41:05Z

@ThomasBurleson it is ready to merge?

ThomasBurleson · 2017-07-30T18:59:30Z

@ardatan - we have a rigorous merge process that includes tests with many applications internal to Google. This will be marked as presubmit. The merge process may take 2-3 work days.

ardatan · 2017-07-30T19:34:05Z

@ThomasBurleson I understand. Thank you for explanation.

ardatan · 2017-07-31T18:12:47Z

@ThomasBurleson I have an idea about pre-rendered flex-layout pages. Now when we are using flex-layout with universal, it only applies default styles than changes by current screen.
But here is the thing, we can prevent that bad-looking issue by changing the approach.
1)We can predict screen size by using user-agent from request(which is available in platform-server/not-implemented but could be)
2)We can apply all styles by using media queries.

What do you think?

mmalerba · 2017-08-02T16:06:21Z

src/lib/flexbox/api/base.ts

-      let findDirection = (styles) => directionKeys.reduce((direction, key) => {
-        return direction || styles[key];
-      }, null);
+      let immediateValue = getDom().getStyle(target, 'flex-direction');


this logic appears twice, move to helper function?

mmalerba · 2017-08-02T16:20:19Z

src/demo-app/app/github-issues/splitter/split-handle.directive.ts

@@ -19,13 +20,14 @@ export class SplitHandleDirective {

  constructor(ref: ElementRef) {
    const getMouseEventPosition = (event: MouseEvent) => ({x: event.movementX, y: event.movementY});
+    const document = ref.nativeElement.ownerDocument;


I see elsewhere you are doing @Inject(DOCUMENT) why not same here?

I will sync the solutions to be the same. Thx.

mmalerba · 2017-08-02T16:25:44Z

src/lib/media-query/match-media.ts

-    let canListen = !!(<any>window).matchMedia('all').addListener;
-    return canListen ? (<any>window).matchMedia(query) : <MediaQueryList>{
+  protected _buildMQL(query: string): MediaQueryList {
+    let canListen = (typeof matchMedia != 'undefined');


can just do canListen = !!matchMedia or not even and just do:

if (matchMedia) { ... } return matchMedia ? ...;

I will use the angular Universal isBrowser() solution.

mmalerba · 2017-08-02T16:29:19Z

src/universal-app/app/responsive-app.css

+  outline: none;
+  -webkit-user-select: none;
+  user-select: none;
+  z-index: 9999;


does the z-index need to be so high?

I just chose an arbitrary large 100.

mmalerba · 2017-08-02T16:33:17Z

src/universal-app/app/responsive-app.css

+  top: 50%;
+  left: -2px;
+  transform: translateX(-50%) rotate(270deg);
+  cursor: col-resize;;


ThomasBurleson · 2017-08-02T18:12:17Z

@mmalerba - all comments fixed. Thank you sir.

@ardatan

Use getDom() for platform-server/universal fixes Now gets document object from platform-browser by DI instead of global. > Thx to @ardatan for PR #346. Fixes #187, #354. Closes #346.

cyrilletuzi · 2017-08-09T13:17:48Z

With last head build, Universal still does not work with Flex Layout, got these errors :

ReferenceError: document is not defined
    at prepareQueryCSS (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:834:34)
    at MatchMedia.exports.MatchMedia.MatchMedia._buildMQL (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:802:9)
    at MatchMedia.exports.MatchMedia.MatchMedia.registerQuery (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:788:28)
    at /node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1063:31
    at Array.forEach (native)
    at MediaMonitor.exports.MediaMonitor.MediaMonitor._registerBreakpoints (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1062:33)
    at new MediaMonitor (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:995:14)
    at MEDIA_MONITOR_PROVIDER_FACTORY (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1525:29)
    at _callFactory (/node_modules/@angular/core/bundles/core.umd.js:9585:26)
    at _createProviderInstance$1 (/node_modules/@angular/core/bundles/core.umd.js:9521:26)
ERROR { Error: Uncaught (in promise): ReferenceError: window is not defined
ReferenceError: window is not defined
    at MatchMedia.exports.MatchMedia.MatchMedia._buildMQL (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:803:27)
    at MatchMedia.exports.MatchMedia.MatchMedia.registerQuery (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:788:28)
    at /node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1063:31
    at Array.forEach (native)
    at MediaMonitor.exports.MediaMonitor.MediaMonitor._registerBreakpoints (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1062:33)
    at new MediaMonitor (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:995:14)
    at MEDIA_MONITOR_PROVIDER_FACTORY (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1525:29)
    at _callFactory (/node_modules/@angular/core/bundles/core.umd.js:9585:26)
    at _createProviderInstance$1 (/node_modules/@angular/core/bundles/core.umd.js:9521:26)
    at resolveNgModuleDep (/node_modules/@angular/core/bundles/core.umd.js:9503:17)
  
rejection:
   { ReferenceError: window is not defined
       at MatchMedia.exports.MatchMedia.MatchMedia._buildMQL (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:803:27)
       at MatchMedia.exports.MatchMedia.MatchMedia.registerQuery (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:788:28)
       at /node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1063:31
       at Array.forEach (native)
       at MediaMonitor.exports.MediaMonitor.MediaMonitor._registerBreakpoints (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1062:33)
       at new MediaMonitor (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:995:14)
       at MEDIA_MONITOR_PROVIDER_FACTORY (/node_modules/@angular/flex-layout/bundles/flex-layout.umd.js:1525:29)

Does not crash the server, but stop rendering all the rest of the page.

ThomasBurleson · 2017-08-09T13:23:58Z

@cyrilletuzi - we need your help to reproduce this issue. Do you have a small demo that we can test locally to produce the same error ? Also please create a new issue for this... or add extra information to issue #354.

cyrilletuzi · 2017-08-09T13:59:15Z

Sorry, it was just a package synchronisation issue (I updated my app flex layout package but not the server one...), all is OK now and I can close the reflect issue.

Is it possible to release a beta.9 for this ? I suppose if it's not still here yet it's because there are other things coming, but as it's a beta release it is not critical to miss some other points and would be better than rely on nightly builds.

ThomasBurleson · 2017-08-09T14:03:37Z

@cyrilletuzi - thx for the follow-up and confirmation/closure of your issue. We hope to release Beta.9 this week.

wis · 2017-08-11T09:39:38Z

great OSS work guys.
happy to hear that material and flex-layout work OOB with Universal now after beta 9 releasi this week 🎉.
@ThomasBurleson what about what @ardatan mentioned above? ;
theres libs for predicting specific device & if Desktop/Mobile from user agent of Express request, we can map it to exact width value for mobile devices and close enough one (1080p?) for desktop, for much better html render.
is there a way to make universal render to specified width?

cyrilletuzi · 2017-08-11T12:13:33Z

@wesamco It's discussed in #373

googlebot added the cla: yes label Jul 29, 2017

ThomasBurleson mentioned this pull request Jul 29, 2017

fix(media-query): fix #187, fixed for platform-server/universal #346

Closed

ThomasBurleson force-pushed the thomas/issue-187 branch 4 times, most recently from c2e03f2 to 4425a1a Compare July 30, 2017 01:00

ardatan reviewed Jul 30, 2017

View reviewed changes

ThomasBurleson force-pushed the thomas/issue-187 branch from 4425a1a to f5cf9e9 Compare July 30, 2017 17:25

ThomasBurleson force-pushed the thomas/issue-187 branch 2 times, most recently from d3acdbe to 1f62f0c Compare July 30, 2017 18:06

ThomasBurleson added this to the v2.0.0-beta.9 milestone Jul 30, 2017

ThomasBurleson added the pr: needs presubmit label Jul 30, 2017

cyrilletuzi mentioned this pull request Jul 31, 2017

Universal issue due to packaging format issue #354

Closed

ThomasBurleson added the ssr label Jul 31, 2017

ThomasBurleson changed the title ~~fix(lib, media-query): use getDom() for platform-server/universal fixes~~ fix(lib, media-query): support angular/universal Aug 2, 2017

mmalerba approved these changes Aug 2, 2017

View reviewed changes

ThomasBurleson force-pushed the thomas/issue-187 branch 2 times, most recently from 534f14c to 56a53f6 Compare August 2, 2017 18:11

mmalerba approved these changes Aug 2, 2017

View reviewed changes

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

ThomasBurleson force-pushed the thomas/issue-187 branch from 56a53f6 to 7311f99 Compare August 3, 2017 19:32

fix(lib, media-query): support angular/universal

f8f56f3

Use getDom() for platform-server/universal fixes Now gets document object from platform-browser by DI instead of global. > Thx to @ardatan for PR #346. Fixes #187, #354. Closes #346.

ThomasBurleson force-pushed the thomas/issue-187 branch from 8f5330e to f8f56f3 Compare August 4, 2017 19:54

tinayuangao merged commit 0f13b14 into master Aug 4, 2017

ppham27 mentioned this pull request Aug 6, 2017

Call to this._getFlowDirection(this.parentElement, true); overwrites flex-direction set in stylesheet #364

Closed

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

angular locked and limited conversation to collaborators Aug 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(lib, media-query): support angular/universal #353

fix(lib, media-query): support angular/universal #353

ThomasBurleson commented Jul 29, 2017 •

edited

Loading

ardatan Jul 30, 2017

ThomasBurleson Jul 30, 2017 •

edited

Loading

ardatan Jul 30, 2017 •

edited

Loading

ThomasBurleson commented Jul 30, 2017

ardatan commented Jul 30, 2017 •

edited

Loading

ThomasBurleson commented Jul 30, 2017

ardatan commented Jul 30, 2017

ThomasBurleson commented Jul 30, 2017

ardatan commented Jul 30, 2017 via email •

edited

Loading

ardatan commented Jul 31, 2017

mmalerba Aug 2, 2017

ThomasBurleson Aug 2, 2017

mmalerba Aug 2, 2017

ThomasBurleson Aug 2, 2017

ThomasBurleson Aug 2, 2017

mmalerba Aug 2, 2017

ThomasBurleson Aug 2, 2017

mmalerba Aug 2, 2017

ThomasBurleson Aug 2, 2017

ThomasBurleson Aug 2, 2017

mmalerba Aug 2, 2017

ThomasBurleson Aug 2, 2017

ThomasBurleson commented Aug 2, 2017

cyrilletuzi commented Aug 9, 2017

ThomasBurleson commented Aug 9, 2017 •

edited

Loading

cyrilletuzi commented Aug 9, 2017

ThomasBurleson commented Aug 9, 2017

wis commented Aug 11, 2017

cyrilletuzi commented Aug 11, 2017

fix(lib, media-query): support angular/universal #353

fix(lib, media-query): support angular/universal #353

Conversation

ThomasBurleson commented Jul 29, 2017 • edited Loading

Choose a reason for hiding this comment

ThomasBurleson Jul 30, 2017 • edited Loading

Choose a reason for hiding this comment

ardatan Jul 30, 2017 • edited Loading

Choose a reason for hiding this comment

ThomasBurleson commented Jul 30, 2017

ardatan commented Jul 30, 2017 • edited Loading

ThomasBurleson commented Jul 30, 2017

ardatan commented Jul 30, 2017

ThomasBurleson commented Jul 30, 2017

ardatan commented Jul 30, 2017 via email • edited Loading

ardatan commented Jul 31, 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

ThomasBurleson commented Aug 2, 2017

cyrilletuzi commented Aug 9, 2017

ThomasBurleson commented Aug 9, 2017 • edited Loading

cyrilletuzi commented Aug 9, 2017

ThomasBurleson commented Aug 9, 2017

wis commented Aug 11, 2017

cyrilletuzi commented Aug 11, 2017

ThomasBurleson commented Jul 29, 2017 •

edited

Loading

ThomasBurleson Jul 30, 2017 •

edited

Loading

ardatan Jul 30, 2017 •

edited

Loading

ardatan commented Jul 30, 2017 •

edited

Loading

ardatan commented Jul 30, 2017 via email •

edited

Loading

ThomasBurleson commented Aug 9, 2017 •

edited

Loading