angular / flex-layout

Provides HTML UI layout for Angular applications; using Flexbox and a Responsive API
MIT License
5.9k stars 771 forks source link

Remove elements/components from DOM instead of CSS-hiding #142

Open netmikey opened 7 years ago

netmikey commented 7 years ago

fxHide and fxShow hide elements via CSS display: none. While CSS-hiding is OK for simple HTML elements, it seems suboptimal for cases where we want to replace a whole component tree with another depending on the display size. In those cases, I would prefer completely removing elements and components from the DOM instead of having them both in the DOM (plus having to maintain their state) and just hiding one of them via CSS.

The Wiki page at https://github.com/angular/flex-layout/wiki/Adaptive-Layouts mentions the following:

Developers can use the following directives to achieve some Adaptive UX goals:

  • fxHide
  • fxShow
  • ngIf

That sounds like I could use ngIf together with FlexLayout breakpoint aliases to achieve what I want, but I can't seem to find an example of how to do that? Is it just not implemented yet? Or could you maybe add an example somewhere in the docs?

Alternatively: what about removing elements from the dom completely on fxHide so that it behaves like ngIf instead of using CSS?

ThomasBurleson commented 7 years ago

The current solution solution for *`ngIf`** is here:

import import {ObservableMediaService} from '@angular/flex-layout/media-query/observable-media-service';

@Component({
  selector : 'my-mobile-component',
  template : `
      <div *ngIf="media.isActive('xs')">
         This content is only shown on Mobile devices
      </div>
      <footer>
         Current state: {{ }}
      </footer>
  `
})
export class MyMobileComponent {
  public state = '';
  constructor( @Inject(ObservableMediaService) public media) {
    media.asObservable()
      .subscribe((change:MediaChange) => {
        this.state = change ? `'${change.mqAlias}' = (${change.mediaQuery})` : ""
      });
  }
}

See https://github.com/angular/flex-layout/issues/144 for details on above import

ThomasBurleson commented 7 years ago

Closing this since it as "not an issue".

netmikey commented 7 years ago

Awesome, @ThomasBurleson, thanks for the update! :)

damsorian commented 7 years ago

I created a workaround to use with ngIf, you can have a Pipe that return a Observable\<boolean> in order to use with the async pipe in the ngIf, like this:

<div *ngIf="['xs','sm'] | fxIf | async">
  This is only for xs and sm.
</div>

FxIf Pipe:

import { Pipe, PipeTransform } from '@angular/core';
import { MediaChange, ObservableMedia } from '@angular/flex-layout';
import { Observable } from 'rxjs';

@Pipe({ name: 'fxIf' })
export class FxIfPipe implements PipeTransform {

  constructor(private media: ObservableMedia) {}

  transform(data: Array<'xs'|'sm'|'md'|'lg'>): Observable<boolean> {
    return this.media.asObservable().map((change: MediaChange) => {
      return data.indexOf(change.mqAlias) > -1;
    });
  }
}

This work for me, I hope it helps you.

ThomasBurleson commented 7 years ago

@damsorian - I think this might be a better solution/enhancement:

import { Component } from '@angular/core';

import { Observable } from 'rxjs';
import { ObservableMedia } from '@angular/flex-layout';

@Component({ 
    selector: 'my-view' 
    template: `
      <div *ngIf="mobile$ | async">
          This is only for xs and sm.
      </div>
    `
})
export class MyView {
  mobile$ : Observable<boolean>;

  constructor(media: ObservableMedia) {
    this.mobile$ = media.asObservableFor(['xs','sm']);
  }

}

This ^ would use a new API observableFor(aliases:Array<string>) to emit true when the aliases activate and false otherwise. Then the standard *ngIf could be easily used with the async pipe.

damsorian commented 7 years ago

@ThomasBurleson - I like the observableFor! what do you think about to use it with a Pipe in order to avoid code repetition in each component? It would look like this:

<div *ngIf="['xs','sm'] | fxIf | async">This is only for xs and sm.</div>
@Pipe({ name: 'fxIf' })
export class FxIfPipe implements PipeTransform {

  constructor(private media: ObservableMedia) {}

  transform(data: Array<'xs'|'sm'|'md'|'lg'>): Observable<boolean> {
    return this.media.asObservableFor(data);
  }
}
ThomasBurleson commented 7 years ago

The pipe idea adds complexity. The angular motto: less is more.

On Sun, Oct 1, 2017, 2:18 PM Damsorian notifications@github.com wrote:

@ThomasBurleson https://github.com/thomasburleson - I like the observableFor! what do you think about to use it with a Pipe in order to avoid code repetition in each component? It would look like this:

<div *ngIf="['xs','sm'] | fxIf | async">This is only for xs and sm.

@Pipe({ name: 'fxIf' })export class FxIfPipe implements PipeTransform {

constructor(private media: ObservableMedia

) {}

transform(data: Array<'xs'|'sm'|'md'|'lg'>): Observable { return this.media.asObservableFor(data); } }

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/angular/flex-layout/issues/142#issuecomment-333403664, or mute the thread https://github.com/notifications/unsubscribe-auth/AAM17b07bUcZ_M-4L7W-tyLTQDiQZkmDks5sn_OAgaJpZM4LwxHR .

damsorian commented 7 years ago

I need to maintain several components with the logic to show and hide elements for mobile, tablet and desktop, some in the same component. And with a Pipe, I don't need to initialize the ObservableMedia and create several variables like $mobile $tablet $desktop in each component every time (imagine 20 components with that logic). For me is too much code repetition, and in my case, the Pipe means 'less is more', but also, I understand your approach, and in many cases is the most intuitive to do. :+1:

netmikey commented 7 years ago

I agree with @damsorian : having to inject media into components and repeat the asObservableFor over and over again doesn't seem like the DRYest solution, especially in a highly responsive WebApp. Also, I like the idea of having this behavior right there in the markup and (for most simple cases) not split up into markup and component. I'd welcome it if its usage was as easy as fxHide and fxShow, and the pipe would certainly achieve this.

From a usability standpoint, *ngIf.<breakpoint alias>="" would seem like the easiest and most readable solution (though I know Angular doesn't want us to implement specialized ngIfs...).

ThomasBurleson commented 7 years ago

Oh I like this ^ idea! PR anyone?

On Fri, Oct 6, 2017, 12:17 AM Mike Meessen notifications@github.com wrote:

I agree with @damsorian https://github.com/damsorian : having to inject media into components and repeat the asObservableFor over and over again doesn't seem like the DRYest solution, especially in a highly responsive WebApp. Also, I like the idea of having this behavior right there in the markup and (for most simple cases) not split up into markup and component. I'd welcome it if its usage was as easy as fxHide and fxShow, and the pipe would certainly achieve this.

From a usability standpoint, *ngIf.="" would seem the easiest and most readable solution (though I know Angular doesn't want us to implement specialized ngIfs...).

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/angular/flex-layout/issues/142#issuecomment-334678018, or mute the thread https://github.com/notifications/unsubscribe-auth/AAM17f-NCGfLggtAouo7EUxF70AHbPBYks5spdQcgaJpZM4LwxHR .

CaerusKaru commented 6 years ago

Would it be satisfactory to add two pipes to replace fxHide and fxShow? If so I can throw a PR together for this and hopefully add it to the next release.

ThomasBurleson commented 6 years ago

@CaerusKaru - I mean that I think adding responsive support to ngIf seems interesting: *ngIf.<breakpoint alias>=""

damsorian commented 6 years ago

@ThomasBurleson @netmikey - There is a problem with the*ngIf.<break> aproach ... we can only apply one structural directive per element, this means the following case is not possible:

<div *ngIf.xs="true" *ngIf.sm="false"></div>
CaerusKaru commented 6 years ago

So here's where we're leaning: we think the best solution is to have fxShow/fxHide compose with ngIf in the same way ngClass.<breakpoint> composes with ngClass. This is in the backlog, but we'll try to prototype it for the next major release.

Ryan-Haines commented 6 years ago

@damsorian thanks for this snippet. I had some minor build issues arise surrounding the type system and wanted to pass along my adaptation for anyone that runs into the same issues:

import { Pipe, PipeTransform } from '@angular/core';
import { MediaChange, ObservableMedia } from '@angular/flex-layout';
import { Observable } from 'rxjs/Observable';

@Pipe({ name: 'fxIf' })
export class FxIfPipe implements PipeTransform {

  constructor(private media: ObservableMedia) {}

  transform(data: Array<'xs'|'sm'|'md'|'lg'>): Observable<boolean> {
    return this.media.asObservable().map((change: MediaChange) => {
      if (!data) {
        return false;
      }
      const mqAlias: any = change.mqAlias;
      return data.includes(mqAlias);
    });
  }
}
intellix commented 6 years ago

I'm doing the following so I can re-use stuff like gt-md:

Using startWith because ObservableMedia doesn't emit straight away in particular scenarios (dunno why, because it always does in StackBlitz). Think I read something about AOT in another issue

import { Pipe, PipeTransform } from '@angular/core';
import { ObservableMedia } from '@angular/flex-layout';
import { Observable } from 'rxjs/Observable';
import { map, startWith } from 'rxjs/operators';

@Pipe({
  name: 'mediaActive',
})
export class MediaActivePipe implements PipeTransform {

  constructor(private media: ObservableMedia) {}

  transform(matcher: string): Observable<boolean> {
    return this.media
      .asObservable()
      .pipe(
        map(() => this.media.isActive(matcher)),
        startWith(this.media.isActive(matcher)),
      );
  }
}
intellix commented 6 years ago

Swapping the above with a structural directive instead:

<div *xcMediaIf="'gt-sm'">Greater than small</div>
import { Directive, Input, TemplateRef, ViewContainerRef, OnDestroy, ChangeDetectorRef } from '@angular/core';
import { ObservableMedia } from '@angular/flex-layout';
import { Subject } from 'rxjs/Subject';
import { combineLatest } from 'rxjs/observable/combineLatest';
import { startWith } from 'rxjs/operators';

@Directive({
  selector: '[xcMediaIf]',
})
export class MediaIfDirective implements OnDestroy {

  private hasView = false;
  private matcher = new Subject<string>();
  private subscription = combineLatest(
      this.matcher,
      this.media.asObservable().pipe(startWith(null)),
    )
    .subscribe(([matcher, _]) => {
      const condition = this.media.isActive(matcher);

      if (condition && !this.hasView) {
        this.viewContainer.createEmbeddedView(this.template);
        this.changeRef.markForCheck();
        this.hasView = true;
      } else if (!condition && this.hasView) {
        this.viewContainer.clear();
        this.hasView = false;
      }
    });

  @Input()
  set xcMediaIf(value: string) {
    this.matcher.next(value);
  }

  constructor(
    private template: TemplateRef<any>,
    private viewContainer: ViewContainerRef,
    private media: ObservableMedia,
    private changeRef: ChangeDetectorRef,
  ) {}

  ngOnDestroy() {
    this.subscription.unsubscribe();
  }
}

Edit 25/02/2018: Had to add ChangeDetectorRef to enforce a markForCheck() so lifecycle methods are called on newly created content

intellix commented 6 years ago

Update for beta.14 using the new MatchMedia, hope it helps someone:

<div *xcMediaIf="'gt-sm'">Greater than small</div>
import { Directive, Input, TemplateRef, ViewContainerRef, OnDestroy, ChangeDetectorRef } from '@angular/core';
import { ObservableMedia, MatchMedia, BreakPointRegistry } from '@angular/flex-layout';
import { Subject, combineLatest } from 'rxjs';
import { startWith, switchMap, tap, map } from 'rxjs/operators';

@Directive({
  selector: '[xcMediaIf]',
})
export class MediaIfDirective implements OnDestroy {
  private hasView = false;
  private matcher = new Subject<string>();
  private subscription = this.matcher
    .pipe(
      map(alias => this.breakpoints.findByAlias(alias).mediaQuery),
      switchMap(mq => {
        return this.matchMedia.observe(mq).pipe(
          map(result => result.matches),
          startWith(this.matchMedia.isActive(mq))
        );
      }),
    )
    .subscribe(matches => matches ? this.createView() : this.destroyView());

  @Input()
  set xcMediaIf(value: string) {
    this.matcher.next(value);
  }

  constructor(
    private template: TemplateRef<any>,
    private viewContainer: ViewContainerRef,
    private breakpoints: BreakPointRegistry,
    private matchMedia: MatchMedia,
    private changeRef: ChangeDetectorRef,
  ) {}

  ngOnDestroy() {
    this.subscription.unsubscribe();
  }

  private createView() {
    if (!this.hasView) {
      this.viewContainer.createEmbeddedView(this.template);
      this.changeRef.markForCheck();
      this.hasView = true;
    }
  }

  private destroyView() {
    if (this.hasView) {
      this.viewContainer.clear();
      this.hasView = false;
    }
  }
}
ThomasBurleson commented 6 years ago

@intellix - careful with your code above. If you use databinding *xcMediaIf="<expression>", you will end up with Zombie subscriptions after the 2nd (or more) values activate.

Try this:

import {Directive, Input, TemplateRef, ViewContainerRef} from '@angular/core';

import {Subject} from 'rxjs/Subject';
import {Observable} from 'rxjs/Observable';
import {combineLatest} from 'rxjs/observable/combineLatest';
import {empty} from 'rxjs/observable/empty';
import {startWith, switchMap, tap, takeUntil} from 'rxjs/operators';

import {untilDestroyed} from '../untilDestroyed';
import {ObservableMedia, MatchMedia, BreakPointRegistry, MediaChange} from '@angular/flex-layout';

@Directive({
  selector: '[xcMediaIf]',
})
export class MediaIfDirective {
   @Input() set fxIf(value: string) {
    this.matcher.next(value);
  }

  constructor(
      private viewContainer: ViewContainerRef,  private templateRef: TemplateRef<any>,
      private breakpoints: BreakPointRegistry,  private matchMedia: MatchMedia ) {
    this.watchMedia();
  }

  /**
   * Watch for mediaChange(s) on new mediaQuery
   */
  private watchMedia(): void {
    const updateView = this.updateView.bind(this);
    const validateQuery = (query) => {    
      const breakpoint = this.breakpoints.findByAlias(query);
      return breakpoint ? breakpoint.mediaQuery : query;
    };

    this.matcher.pipe(
      untilDestroyed(this),      
      switchMap(val => {
        const query = validateQuery(val);
        return query ? this.matchMedia.observe(query) : empty();
      })
    )
    .subscribe(updateView);   
  }

  /**
   * Create or clear view instance
   */
  private updateView(change: MediaChange) {
    if (change.matches && !this.viewRef) {
      this.viewRef = this.viewContainer.createEmbeddedView(this.templateRef);
    } else if (!change.matches && this.viewRef) {
      this.viewContainer.clear();
      this.viewRef = undefined;
    }
  }

  private viewRef = undefined;
  private matcher = new Subject<string>();
}
intellix commented 6 years ago

Are you sure? I'm not seeing it but might be missing something.

Newly detected values through the expression should next through the Subject which switchMaps into an inner breakpoint observable. The whole chain and it's inner observables only get subscribed to once as far as I can see and it's all cleaned up through OnDestroy

ThomasBurleson commented 6 years ago

@intellix - 1) Good: you are switching from matcher to matchMedia.observe(query) and subscribe. 2) Good: you cache the last subscription to clear on ngOnDestroy() 3) Zombie: previous subscriptions (if any) are never cleared.

ThomasBurleson commented 6 years ago

@intellix - I was not thinking clearly. You are correct that (in your case) the subscription remains constant and you only have (1) and (2). So you do not have zombies because you create it one time only.

You would have zombies if you created a new subscription for each match change.

CaerusKaru commented 5 years ago

I've been looking at this again with the refactor of #900 looming. And the one question I have is: what does this look like with SSR? With fxHide/fxShow, we could just inject the display property for each media query, but we can't do this here because by nature we're changing the DOM itself.

My issue with this is that if people are developing applications, they may see this as a superior solution to fxShow/fxHide, and get a less-than-optimal result when rendering on the server. Is there another option I'm overlooking?

manzonif commented 4 years ago

@CaerusKaru, What do you think about the React approach? https://github.com/artsy/fresnel