quentinlampin / ngx-openlayers

Angular2+ components for Openlayers 4.x
Mozilla Public License 2.0
137 stars 98 forks source link

ol.control implementation #45

Closed achimha closed 7 years ago

achimha commented 7 years ago

I started looking into the unfinished control implementations.

There are a few questions. First of all, I don't think you can derive the classes from the OL classes as this breaks the @Input() parameter setting. So I changed it to the instance method used elsewhere. Next, the question is whether to use transclusion with <ng-content>. I couldn't get it to work so I went for this solution which uses the component's DOM element:

@Component({
  selector: 'aol-control-mouseposition',
  template: ``
})
export class ControlMousePositionComponent implements OnInit, OnDestroy {
  instance: control.MousePosition;
  @Input() coordinateFormat: CoordinateFormatType;
  @Input() projection: ProjectionLike;
  target: Element;

  constructor(
    @Host() private map: MapComponent,
    private element: ElementRef
  ) {
  }

  ngOnInit() {
    this.target = this.element.nativeElement;
    // console.log('ol.control.MousePosition init: ', this);
    this.instance = new control.MousePosition(this);
    this.map.instance.addControl(this.instance);
  }

  ngOnDestroy() {
    // console.log('removing aol-control-mouseposition');
    this.map.instance.removeControl(this.instance);
  }
}

In order to style it, you will need the piercing CSS selector ( :host /deep/) which is probably OK. Any thoughts on how to do the controls implementation?

achimha commented 7 years ago

With a wrapper for the <ng-content>, it actually does work. Here's my generic ol.Control implementation that displays the content. Styling with selector piercing.

import { Component, ElementRef, Host, OnDestroy, OnInit, ViewChild } from '@angular/core';
import { control } from 'openlayers';
import { MapComponent } from '../map.component';

@Component({
  selector: 'aol-control',
  template: `<div #wrapper><ng-content></ng-content></div>`
})
export class ControlComponent implements OnInit, OnDestroy {
  public componentType: string = 'control';
  instance: control.Control;
  element: Element;

  @ViewChild('wrapper') wrapper: ElementRef;

  constructor(
    @Host() private map: MapComponent
  ) {
  }

  ngOnInit() {
    this.element = this.wrapper.nativeElement.children[0];
    this.instance = new control.Control(this);
    this.map.instance.addControl(this.instance);
  }

  ngOnDestroy() {
    this.map.instance.removeControl(this.instance);
  }
}
quentinlampin commented 7 years ago

About the subclassing of ol.control classes : you're right. We moved away from this design in every other component but I haven't found the time to change the controls yet.

Regarding the transclusion and the wrapper solution, I'm not sure I understand what you're trying to achieve. Could you provide an example so I understand what's at stake and come back with meaningful ideas ?

achimha commented 7 years ago

My (rather big now) PR has the control reimplementation included.

With the control class copied above, I can do this:

     <aol-control>
            <div id="settingscontrol" class="ol-control ol-unselectable">
                <button (click)="showSettings = true">
                    <i class="fa fa-bars"></i>
                </button>
            </div>
        </aol-control>

The only problem I see is that I get an additional wrapper div. I don't know a way around that as I can't get hold of the <ng-content> without an element to reference. It's probably OK but I don't like the extra div. You can see in my class that I get hold of the #wrapper element and then pass its first child to the OL API.

quentinlampin commented 7 years ago

Hmm, I'm not a fan of the wrapper div solution either.

Have you tried passing the ElementRef to the component constructor and then accessing the transcluded content like this ?

 constructor(
    @Host() private map: MapComponent,
    private element: ElementRef
  ) {
  }
[...]

this.element.nativeElement.childNodes[0]
achimha commented 7 years ago

Actually this works as well. Thanks!

achimha commented 7 years ago

Came up with another issue with this approach (taking the first child). I created a an implementation of ol.Overlay. Among its parameters is the map position for which I used the existing <aol-coordinate>. Now the question is how to differentiate between the content of the overlay (inserted via transclusion) and the coordinate. Example markup:

        <aol-overlay>
            <div><h1>THIS IS AN OVERLAY</h1></div>
            <aol-coordinate [x]="center[0]" [y]="center[1]" [srid]="'EPSG:4326'"></aol-coordinate>
        </aol-overlay>

The approach taken is to get an ElementRef and pass the first child to the OL control. Obviously it works (coordinate is the second) but is this a good approach?

achimha commented 7 years ago

What about enumerating children until we get the first one that does not start with aol-? Uglier? Less ugly? 😄

quentin-ol commented 7 years ago

;)

We could also make use of HTML element IDs (#ugly) or CSS classes but that's .notpretty.

We could also enforce a "unique child" policy but that feels wrong (especially the name :/) Puns aside, I believe we should do multiple-slots content projection and write an "aol-content" directive (not component) to discriminate the content from the rest.

Opinions?

Interesting read on transclusion/projection: https://toddmotto.com/transclusion-in-angular-2-with-ng-content -------- Message d'origine -------- De : Achim Hasenmueller notifications@github.com Date : 13/02/2017 18:48 (GMT+01:00) À : quentin-ol/angular2-openlayers angular2-openlayers@noreply.github.com Cc : Subscribed subscribed@noreply.github.com Objet : Re: [quentin-ol/angular2-openlayers] ol.control implementation (#45)

What about enumerating children until we get the first one that does not start with aol-? Uglier? Less ugly? 😄

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/quentin-ol/angular2-openlayers/issues/45#issuecomment-279467200, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AOYyYuVFLHpqIjJIuJDa3a6SEa9znMM5ks5rcJd2gaJpZM4L-LHJ.


Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you.

achimha commented 7 years ago

I like the <aol-content> approach. That's at least clean. I'll implement it like this.

achimha commented 7 years ago

Implemented and documented as part of my latest PR.