Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue when ApplicationView is not rendered because of an empty app list #465

Merged
merged 3 commits into from
May 22, 2017

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented May 22, 2017

Fixes #461

@martinRenou martinRenou changed the title Fixes #461 Fix issue when ApplicationView is not rendered because of an empty app list May 22, 2017
let iframe = this.$el.querySelector('iframe');
if(iframe !== null) {
iframe.focus();
if(this.$el.querySelector !== undefined && // In case $el is not rendered
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are not using the right approach. if the element is not rendered, you should check if the element is not rendered. You are probing the existence of a routine that happens to be correlated to the existence of the element, but it's not the thing that is the core issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are absolutely right

@codecov-io
Copy link

codecov-io commented May 22, 2017

Codecov Report

Merging #465 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #465   +/-   ##
=======================================
  Coverage   95.33%   95.33%           
=======================================
  Files          88       88           
  Lines        4077     4077           
  Branches      259      259           
=======================================
  Hits         3887     3887           
  Misses        138      138           
  Partials       52       52

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b167462...e356af6. Read the comment docs.

@martinRenou martinRenou dismissed stefanoborini’s stale review May 22, 2017 17:24

We agreed on how to fix it

@martinRenou martinRenou merged commit e729787 into master May 22, 2017
@martinRenou martinRenou deleted the fix_el_issue branch May 22, 2017 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants