miguelcobain / ember-paper

The Ember approach to Material Design.
http://miguelcobain.github.io/ember-paper
MIT License
889 stars 331 forks source link

"Cannot read property 'appendChild' of null" when running integration test with PaperMenu #1151

Closed calvinps closed 3 years ago

calvinps commented 4 years ago

I have a component that renders a PaperMenu component. After running an integration test for my component in which I render the PaperMenu component and click on the trigger, my test suite fails. The source of the issue is that the animateOut function in PaperMenu::Content the element doesn't have a "parentElement" and the function crashes at the end of the test. I've tried unsuccessfully to find a way to destroy the PaperMenu gracefully.

Ember version: 3.18 Ember Paper version: 1.0.0-beta.30

My component:

<PaperMenu @isOpen={{this.isOpen}} @position="target-right target" as |menu|>
  <menu.trigger>
    <PaperButton @class="menu-btn" @onClick={{null}} @iconButton={{true}}>
      <ProfilePicture @user={{this.user}} @class="user-icon" @height={{42}} />
    </PaperButton>
  </menu.trigger>
  <menu.content as |content|>
    {{#each @items as |item|}}
      <content.menu-item @onClick={{action "selected" item}}>
        <p class="tool-bar-menu-item-title">
          {{item.title}}
        </p>
        {{#if item.icon}}
          <PaperIcon @icon={{item.icon}} @class="md-menu-align-target" />
        {{/if}}
      </content.menu-item>
    {{/each}}
  </menu.content>
</PaperMenu>

My test:

import { module, test } from "qunit";
import { setupRenderingTest } from "ember-qunit";
import {
  click,
  findAll,
  render,
  settled
} from "@ember/test-helpers";
import hbs from "htmlbars-inline-precompile";

module("Integration | Component | tool-bar/menu", function (hooks) {
  setupRenderingTest(hooks);

  hooks.beforeEach(function () {
    this.user = this.owner.lookup("service:store").createRecord("user", {
      profilePicture:
        "https://xxxxx.s3.amazonaws.com/media/190304/conversions/blob-thumb.jpg",
      fullName: "John Doe",
    });
  });

  test("it renders", async function (assert) {
    this.set("items", [
      {
        title: "Home",
        transitionTo: "get-started",
      },
      {
        title: "Settings",
        transitionTo: "settings",
      },
      {
        title: "Logout",
        transitionTo: "logout",
      },
    ]);

    await render(
      hbs`<ToolBar::Menu @items={{this.items}} @user={{this.user}} />`
    );

    assert.equal(
      this.element.querySelector("img").getAttribute("src"),
      this.user.profilePicture
    );

    await click("button.menu-btn");
    await settled();
    let selectors = findAll("md-menu-item");
    assert.dom(selectors[0]).hasText("Home");
    await click("button.menu-btn");
  });
});

A quick fix would be to simply return early in the animateOut if a parentElement isn't found on the given element

gabrielcsapo commented 4 years ago

I just ran into this same problem.

MarcoUmpierrez commented 3 years ago

It seems that will-destroy modifier isn't including parentElement to the element and it could be related to this issue

Adding this prevents the error:

if (! parentElement) {
  parentElement = document.getElementById('ember-basic-dropdown-wormhole');
}

I don't know if it's worth a PR

toovy commented 3 years ago

@miguelcobain the fix @MarcoUmpierrez suggested actually works, seems a bit "hacky", but as the discussion in https://github.com/emberjs/ember.js/issues/18795 is still not resolved. What do you think?

chbonser commented 3 years ago

The 'hacky' option here has allowed me to get our app to 3.20. I'll be referencing this commit directly until we find a better solution: https://github.com/teamtopia/ember-paper/commit/abfdf1441a092e9d9a10a729397a8db3eb529054

miguelcobain commented 3 years ago

@chbonser do you mind creating a PR with your workaround? I don't see a good way to solve this.

chbonser commented 3 years ago

I'll post one this afternoon.

On Tue, Sep 1, 2020 at 9:56 AM Miguel Andrade notifications@github.com wrote:

@chbonser https://github.com/chbonser do you mind creating a PR with your workaround? I don't see a good way to solve this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/miguelcobain/ember-paper/issues/1151#issuecomment-684915979, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHZU5WFMRL5DKIYRNYWBULSDUDSZANCNFSM4NMQNDOQ .

miguelcobain commented 3 years ago

I added the work around. Should be in the next release. Thanks everyone for reporting!

miguelcobain commented 3 years ago

Please try 1.0.0-beta.32 and reopen if issue persists.

MarcoUmpierrez commented 3 years ago

This fix prevent the error in the console but it doesn't fix the problem of the menu appearing magically from the left side of the screen the second time you click on it. To fix it is necessary to specify the parent, that's why I added the line that looks for ember-basic-dropdown-wormhole.