flex-layout icon indicating copy to clipboard operation
flex-layout copied to clipboard

Remove elements/components from DOM instead of CSS-hiding

Open netmikey opened this issue 8 years ago • 24 comments

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?

netmikey avatar Jan 29 '17 11:01 netmikey

  • *ngIf.<breakpoint alias>="" is not yet supported.
  • [ngStyle.<alias>]="" is not yet supported.
  • [ngClass.<alias>]="" is not yet supported.

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 avatar Jan 30 '17 00:01 ThomasBurleson

Closing this since it as "not an issue".

  • Other feature requests are available.
  • Updated the WIKI: https://github.com/angular/flex-layout/wiki/Adaptive-Layouts

ThomasBurleson avatar Jan 30 '17 00:01 ThomasBurleson

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

netmikey avatar Jan 30 '17 18:01 netmikey

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.

damsorian avatar Sep 26 '17 18:09 damsorian

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

ThomasBurleson avatar Oct 01 '17 19:10 ThomasBurleson

@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);
  }
}

damsorian avatar Oct 01 '17 20:10 damsorian

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

On Sun, Oct 1, 2017, 2:18 PM Damsorian [email protected] 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:

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 .

ThomasBurleson avatar Oct 02 '17 03:10 ThomasBurleson

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:

damsorian avatar Oct 02 '17 04:10 damsorian

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

netmikey avatar Oct 06 '17 07:10 netmikey

Oh I like this ^ idea! PR anyone?

On Fri, Oct 6, 2017, 12:17 AM Mike Meessen [email protected] 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 .

ThomasBurleson avatar Oct 06 '17 22:10 ThomasBurleson

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.

CaerusKaru avatar Jan 03 '18 08:01 CaerusKaru

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

ThomasBurleson avatar Jan 03 '18 15:01 ThomasBurleson

@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>

damsorian avatar Jan 03 '18 16:01 damsorian

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.

CaerusKaru avatar Jan 06 '18 22:01 CaerusKaru

@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);
    });
  }
}

Ryan-Haines avatar Jan 18 '18 20:01 Ryan-Haines

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 avatar Jan 23 '18 10:01 intellix

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 avatar Jan 23 '18 12:01 intellix

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

intellix avatar Apr 07 '18 12:04 intellix

@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>();
}

ThomasBurleson avatar Apr 08 '18 12:04 ThomasBurleson

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

intellix avatar Apr 08 '18 13:04 intellix

@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 avatar Apr 08 '18 13:04 ThomasBurleson

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

ThomasBurleson avatar Apr 08 '18 13:04 ThomasBurleson

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?

CaerusKaru avatar Dec 09 '18 05:12 CaerusKaru

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

manzonif avatar Oct 02 '20 10:10 manzonif