Updated patternfly to v3.23

[skateman]

Merge together with: ManageIQ/manageiq#13940
Related issue: #400

Closes #400

[skateman]

@miq-bot assign @himdel

[himdel]

Hakiri issues are not caused by this PR and look like false positives .. @martinpovolny ?

[himdel]

The notification icon is completely gone, and the indicator is offset..

fixed by 3.21

[himdel]

Ok, this will be problematic...

master:

(ignore the black rectangle)

pf320:

(diff:
diff
)

• the font has changed (which is possibly OK, idk)
• quadicon quads have moved a few pixels to the bottom
• quadicon checkboxes have moved upwards
• the search bar is about 20% wider
• tree items have moved a few pixels upwards

[himdel]

vm_infra list view is too wide now:

nope, happens on master too

[himdel]

all topology screens are broken

fixed by 3.21

[himdel]

The bottom accordion looks cut off

[himdel]

In fact.. all the angular screens are broken :)

Will investigate more...

fixed by 3.21

[yaacov]

They have a fix for something similar:
patternfly/angular-patternfly#422
It looks like we will have to wait for a new release :-(

[yaacov]

@skateman @himdel https://github.com/patternfly/angular-patternfly/releases/tag/v3.21.0 is out 🎂

[mzazrivec]

@karelhala What's your take on this one? Is it ok to go?

[yaacov]

are we missing
"angular-drag-and-drop-lists": "2.0.0",
line ?
I can't find it , but I do not know if we need it :-) see:
https://github.com/patternfly/angular-patternfly/pull/422/files

[skateman]

It's there, I'm just not keeping the stuff in master 😉

[yaacov]

👍 thanks

[karelhala]

Can confirm that adding //= require angular-drag-and-drop-lists to application.js fixed the problems
#402 (comment) (missing notification icon)
#402 (comment) (error in angular screen)
#402 (comment) (broken angular screens)

[martinpovolny]

Reopening for Hakiri.

[himdel]

So.. I've tested in UI, including the production branch.. and it seems to work completely now, so...

@martinpovolny can you merge both PRs please? (ManageIQ/manageiq#13940)

(I still hate the new font, but .. nothing is really broken, so..)

EDIT: don't, not yet..

[himdel]

So, @skateman this will be harder.

Basically, you have to do this: https://github.com/angular-ui/bootstrap/wiki/Migration-guide-for-prefixes

Every single place where we use ui-bootstrap directives is broken now, and these need to be renamed.

( https://github.com/patternfly/angular-patternfly/releases?after=v3.17.0 - 3.16.0 updated to newer version of angular-ui-bootstrap)

(You can check by going to Compute > Containers > Provider - pick one - toolbar Monitoring > Ad hoc Metrics .. the tenant input should have a typeahead, but doesn't with new PF)

[martinpovolny]

What I don't understand here is: Why would someone do such breaking changes in a minor version?

I would expect patternly to be in maintenance mode and do only fixes. But I probably misunderstood the plans there :-(

ping @serenamarie125

[serenamarie125]

@martinpovolny Have you guys pinged anyone in PatternFly ? I agree that this doesn't make sense in a minor release. @jeff-phillips-18 are you aware of this change? I'm happy to contact PatternFly on this matter, but want to understand more details if possible

[himdel]

The good thing is that if we force angular-ui-bootstrap back to 0.13.*, we get warnings for these things (but risk incompatibilities).
Another good thing, only dropdown-menu can be a class, the rest is restricted to elements or attributes only (thus hopefully easier to search for)...

[himdel]

(note to self: https://gist.github.com/himdel/d6467c0da7ef670952dcc667b4b5fe58 .. angular-ui-bootstrap comes from gem and bower now)

[jeff-phillips-18]

@martinpovolny @serenamarie125 My understanding is with angular bootstrap 0.14.x you can use either the uib prefixes or continue with the older style so this is not a breaking change.

[martinpovolny]

@himdel : can we keep using the old style?

[himdel]

Not sure, I tried and it didn't work, @skateman says the same. But I'll have to recheck, maybe there was a some dependency conflict that caused bower to install a much newer version...

[miq-bot]

Checked commit skateman@8e4235c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks good. 🍰

[skateman]

Seems like all the errors are gone since 3.23.0...

[epwinchell]

@himdel Tested. This one looks good.

[himdel]

So.. the info icon looks a bit broken.

But that's the only issue I can find, so.. yeah.. @martinpovolny feel free to merge please... (together with ManageIQ/manageiq#13940)

(We can always replace by glyphicon glyphicon-info-sign (or fa fa-info) and add .alert > .glyphicon {font-size: 22px; position: absolute; left: 13px; top: 10px; } to our css.)

EDIT: should be fixed in the next release - patternfly/patternfly#621

[moolitayer]

@yaacov BTW I see this is already in fine:
https://github.com/ManageIQ/manageiq-ui-classic/commits/fine?after=72088546a212be2bf486641a9c5b2b70a0f6e073+34

[skateman]

Yup, this was merged before the fine branch was made

[yaacov]

@yaacov BTW I see this is already in fine:

@moolitayer @skateman thanks, all the regression in the ad-hoc metrics page, related to patternfly v3.23 update, are fixed merged and backported to fine 🥇

p.s.
I can now remove the hack I did for the charts to work with older versions of patternfly, I forgot about that :-)

[skateman]

@yaacov Fine 👍