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

A dep no longer needed, causes loading of angular too many times error #1295

Conversation

AllenBW
Copy link
Member

@AllenBW AllenBW commented Nov 30, 2017

this dep was introduced to resolve this issue, angular/angular.js#8934

It has since been closed, we no longer need this dep, pruning it to remove the following error

Here was the issue, console error "loading angular too many times"

screen shot 2017-11-30 at 2 43 45 pm

This fix resolves the issue, dep is no longer required, svgs show up just fine in both prod and dev (rh os img is an svg in the screenshots)

screen shot 2017-11-30 at 2 41 59 pm

screen shot 2017-11-30 at 2 51 16 pm

screen shot 2017-11-30 at 2 50 25 pm

@AllenBW AllenBW added this to the Sprint 75 Ending Dec 11, 2017 milestone Nov 30, 2017
@AllenBW AllenBW requested review from ohadlevy and himdel December 1, 2017 17:58
@miq-bot
Copy link
Member

miq-bot commented Dec 1, 2017

Checked commit AllenBW@827e1e8 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@AllenBW AllenBW assigned ohadlevy and himdel and unassigned ohadlevy Dec 4, 2017
@AllenBW AllenBW requested review from chalettu and removed request for ohadlevy December 4, 2017 15:13
Copy link
Contributor

@chalettu chalettu left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@himdel
Copy link
Contributor

himdel commented Dec 5, 2017

The angular issue was closed with the caveat that you need to be using $locationProvider.html5Mode({enabled: true, requireBase: false}); (or not using the <base> tag).

Since we do neither, we're still affected by that issue.

But.. looking at the original, looks like this only effects svg properties with URLs inside them - both clip-path and mask can take an URL.

Our SVGs don't seem to be using mask at all, but they are using clip-path with URL:

client/assets/images/providers/vendor-openstack_infra.svg
15:			<g clip-path="url(#SVGID_2_)" enable-background="new    ">

But.. the url is a fragment in the same CSS, which probably means we're not affected by the issue after all :).


Would love to be sure though, @AllenBW where do I find a place where vendor SVGs are actually used?

I see a reference in resource-details but haven't quite been able to figure out the right screen.

(All I see is images coming from the API.)

@himdel
Copy link
Contributor

himdel commented Dec 5, 2017

.. never mind, http://localhost:3001/services/10000000000025/resource=10000000001130 works for me :).

It's: Services > pick a service with more than 0 vms, of type openstack > click on the detail screen > click on the VM.

@himdel
Copy link
Contributor

himdel commented Dec 5, 2017

LGTM, merging :)

(but if we ever add a svg with a clip-path that's an actual url, this may break)

Copy link
Contributor

@himdel himdel left a comment

Choose a reason for hiding this comment

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

LGTM, we have no affected SVGs

@himdel
Copy link
Contributor

himdel commented Dec 5, 2017

@AllenBW Apparently, you need to add a review for yourself before I can merge :/ - code owner.

@himdel himdel merged commit 50fd121 into ManageIQ:master Dec 5, 2017
@AllenBW AllenBW deleted the bug/master/#0000-warning-loading-angular-too-many branch December 6, 2017 11:33
simaishi pushed a commit that referenced this pull request Dec 11, 2017
…g-angular-too-many

A dep no longer needed, causes loading of angular too many times error
(cherry picked from commit 50fd121)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit ecd81c8d44c20a5c92819da05815db8c3cbcd315
Author: Martin Hradil <[email protected]>
Date:   Tue Dec 5 17:12:55 2017 +0000

    Merge pull request #1295 from AllenBW/bug/master/#0000-warning-loading-angular-too-many
    
    A dep no longer needed, causes loading of angular too many times error
    (cherry picked from commit 50fd12110a45f19f91141947fa2020e5d0edad12)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants