Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Ensure charmbrowser is fully removed on destroy. #354

Merged
merged 1 commit into from May 30, 2014

Conversation

kadams54
Copy link
Contributor

QA

With the il flag.

  1. Drag a service to the canvas
  2. Click on the service and open the inspector
  3. Close the inspector
  4. Inspect the HTML of the sidebar

You should only see one set of charmbrowser HTML. You should not see anything like this:

<div id="yui_3_11_0_1_1401404489618_1020" ...
    <div class="search-widget"...
    <div class="charm-list"...
<div id="yui_3_11_0_1_1401404489618_1020" ...
    <div class="search-widget"...
    <div class="charm-list"...

window.flags.il = true;
charmBrowser.render();
charmBrowser.destroy();
assert.equal(charmBrowser.get('container').getDOMNode(), null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert.isNull?

@jujugui
Copy link
Contributor

jujugui commented May 30, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1128/

@frankban
Copy link
Member

Code looks good and QA is ok following the instructions.
While QAing I noticed the charm/bundle results in the sidebar are reloaded each time the inspector is closed. This is suboptimal from both UX and resource loading perspectives. Not knowing the background for this change, I guess this can be tackled in another branch.
👍

@kadams54
Copy link
Contributor Author

Thanks for the review, QA, and feedback! I'll plan on tackling the reloading next. :shipit:

@jujugui
Copy link
Contributor

jujugui commented May 30, 2014

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge

jujugui added a commit that referenced this pull request May 30, 2014
### QA

With the `il` flag.

1. Drag a service to the canvas
2. Click on the service and open the inspector
3. Close the inspector
4. Inspect the HTML of the sidebar

You should only see one set of charmbrowser HTML. You should **not** see anything like this:

    <div id="yui_3_11_0_1_1401404489618_1020" ...
        <div class="search-widget"...
        <div class="charm-list"...
    <div id="yui_3_11_0_1_1401404489618_1020" ...
        <div class="search-widget"...
        <div class="charm-list"...
@jujugui jujugui merged commit dfdd741 into juju:develop May 30, 2014
@kadams54 kadams54 deleted the charmbrowser-rerender branch May 30, 2014 15:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants