Add icon DOM Element with absolute positioning code criticism


I’d like to get some feedback on some code I was playing around with. I’ve been curious about doing less form-like ionic apps, so I tried creating a simple app that places icons arbitrarily under wherever the user has tapped the screen. I wanted the last added icon to be red, and all older ones to be green.

The plunker:

Here’s the html:

<link rel="stylesheet" href="">
<ion-header id="id">
<ion-content id="body" padding (click)="getCoordinates($event)">
  <ion-icon  [hidden]="!showHeart" id="heart" [ngStyle]="expression"  name="heart"></ion-icon>

and the TS file:

import { Component } from '@angular/core';
import { IonicPage, NavController, NavParams } from 'ionic-angular';

  selector: 'page-scratch',
  templateUrl: 'scratch.html',
export class ScratchPage {
  private x;
  private y;
  private expression;
  private showHeart = false;

  constructor(public navCtrl: NavController, public navParams: NavParams) {

  getCoordinates(event) {
    this.showHeart = true;
    //get coordinates of click
    this.x = event.clientX;
    this.y = event.clientY;
    //get data to position red heart correctly
    var header = document.getElementById("id");
    var heart = document.getElementById("heart")
    var header_height = header.offsetHeight;
    var heart_height = heart.offsetHeight;
    var heart_width = heart.offsetWidth;
    //red heart position (centered)
    var y_pos = this.y - header_height - heart_height / 2
    var x_pos = this.x - heart_width / 2;
    //Data for [ngStyle] binding
    this.expression = { color: "red", position: "absolute", top: y_pos + "px", left: x_pos + "px" }

    //Code to leave green hearts behind.
    let beer = "<i class=\"fa fa-heart\" style=\"position: absolute; top:" + (this.y - 10) + "px; left: " + (this.x - 10) + "px; color: green;\"  ></i>"
    var body_element = document.getElementById('body');
    body_element.insertAdjacentHTML('afterbegin', beer);



And a picture of how it looks in ionic serve:

The red heart is the last pressed position and it uses the ionic icons set… but I was unable to use ionic icons with the .insertAdjacentHTML method. (which is why I added the font awesome icons in order to bypass the need for the ionic icons.)

Anyhow, the code runs, but I’d like to know if there would be a better way to do this. (The additional of the font awesome icons seems particularly “hacky” but I couldn’t find a way around it.) Are there better ways of creating DOM elements (the green hearts) that might me more “Ionic” in nature? Are there better ways to do this in general?

Anyway, I appreciate any insight or criticism that you might have… and thanks!


Absolutely. Making a list of backing objects in the controller containing the relevant properties (heart color, style including coordinates), looping across that list in the template with ngFor, and binding the relevant properties such as [style]. Direct DOM manipulation is strongly frowned upon in Angular apps.


so, I’m going to make another plunker with this method. Just to make sure I understand, this would look something like:

on click event :
-Make new heart object {name: “heart”, top: this.y_coor, left: this.x_cood, color: heart.color}
-push heart object to heartArray

in template:
<ion-icon *ngFor=“let heart of heartArray” name = [ngStyle]=“position:absolute,, left: heart.left”>

like that?

I started reading more on direct DOM manipulations being poor practice and found this excerpt from this reddit thread that makes it sound like the biggest issue is keeping track of the state machine that it introduces:

Your app starts off in some initial state A, let’s say the HTML in the page already.

Then it changes to state B, so you change some bits of the DOM to reflect this.

Then your app changes to state C. So was it in state A or state B before this? What parts do I need to change?

Then after that, how do I get back to state A?

Is that the biggest reason to stay away from DOM manipulations?

I’m trying to get to the next level programming within the ionic framework, but I’ve found it difficult to separate what I CAN do with what I SHOULD do. I’m currently trying to absorb this article to try and add more powerful tools to my tool belt. Reading that, it seems like structural directives (like *ngFor) and components are the way to go for (controlled) DOM manipulations. Is this correct?

EDIT: another interesting quote from the Reddit thread:

The DOM is your VIEW. If you directly (imperatively) modify it then the DOM has also become your MODEL.

When there is any problem in the DOM then you must search every single one of your imperative functions which may have directly manipulated it.


Yes on this part.

Two choices here, so use whatever feels more natural to you:

  • split the style bits up and bind each of them like []=""
  • build the entire style string up as a monolithic property and do [ngStyle]=""

I think of it as a blurring of the separation of concerns. The point of using a framework like Angular in the first place is that you let it handle DOM manipulation. That means you need to get out of its way and let it do its work. The chef in a restaurant doesn’t want the food buyer in there moving the pots around.


OK… I made a new plunker with the updated code. That for the push in the right direction rapropos… the code is WAY cleaner:


<ion-header id="header">
<ion-content padding (click)="getCoordinates($event)">
  <ion-icon *ngFor="let heart of heartArray;" name={{}} [style.color]=heart.color [style.position]=heart.position [] [style.left]=heart.left >Beer</ion-icon>

TS file:

import { Component } from '@angular/core';
import { IonicPage, NavController, NavParams } from 'ionic-angular';

  selector: 'page-home',
  templateUrl: 'app/'
export class HomePage {
  private heartArray = []
  private showHeart = false;
  constructor(public navCtrl: NavController, public navParams: NavParams) {
  getCoordinates(event) {>{heart.color="green"})
    let header = document.getElementById("header");
    let header_height = header.offsetHeight;
    let x = (event.clientX - 10)+"px";
    let y = (event.clientY - header_height - 10)+"px";
    // build heart object
    let newHeart = {
      name: "heart",
      top: y,
      left: x,
      position: "absolute"