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

Patternfly upgraded to v3.9.0 #10509

Merged
merged 4 commits into from Sep 1, 2016
Merged

Conversation

skateman
Copy link
Member

Now we're back with the proper gem version!
@himdel @epwinchell please review

@epwinchell
Copy link
Contributor

Looks good

@epwinchell
Copy link
Contributor

@miq-bot add_label ui

@miq-bot miq-bot added the ui label Aug 16, 2016
@himdel
Copy link
Contributor

himdel commented Aug 17, 2016

@skateman seems like there was a change in patternfly where selectpicker doesn't set bs-select-hidden on the original select => failing tests ;)

The vmdb failure looks rather unrelated otoh..

@himdel
Copy link
Contributor

himdel commented Aug 18, 2016

I'm looking at the test failures but...

toolbarz


EDIT:

The bower resolutions problem - hoping to fix that by something like bower/bower#2345 (but not sure if bower is still alive enough for this to get in..)

The bs-select-hidden test fail is because previously, bootstrap-select would hide the select and add its own div right after, whereas now, it puts the select inside its own div, making it hidden by .boostrap-select > select instead of .bs-select-hidden...

@skateman
Copy link
Member Author

😢 😢 😢 😢 😢 😢 😢 😢 😢 😢 😢 😢 😢 😢 😢 😢
@epwinchell?

@mzazrivec mzazrivec changed the title Patternfly upgraded to v3.8.1 [WIP] Patternfly upgraded to v3.8.1 Aug 18, 2016
@mzazrivec mzazrivec added the wip label Aug 18, 2016
@himdel
Copy link
Contributor

himdel commented Aug 18, 2016

Updated the tests to reflect the new selectpicker structure (and cleaned up those elem[0][2] in favor of jquery selectors and descriptive variables).

The toolbar is still broken though..

@himdel
Copy link
Contributor

himdel commented Aug 18, 2016

.. to track the problems ..

  • failing tests caused by different selectpicker structure
  • view toolbar not being positioned to the right
  • missing margin after separator (also in master, fixed by Replace ruby generated toolbar with angular's #9753)
  • uneven icon widths in toolbar dropdown causing text not to be aligned (also in master)
  • download_choice having moved to the right (eg /service/explorer) - seems like the logic to move right should only apply to buttons with id =~ /^view_/ - (should be fixed in ui-components 0.0.6)
  • toolbar too tall (fixed in cb71542)
  • active menu item has a top border on hover (patternfly bug - fixed in the next release)

@skateman
Copy link
Member Author

Let's wait for the angular toolbar to be merged, then we should upgrade directly to 3.9.0. @himdel @epwinchell ?

@epwinchell
Copy link
Contributor

@skateman I totally agree.

@himdel
Copy link
Contributor

himdel commented Aug 18, 2016

Sure, no rush :) might be easier to fix the view toolbar after.

@miq-bot
Copy link
Member

miq-bot commented Aug 22, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@himdel
Copy link
Contributor

himdel commented Aug 29, 2016

@skateman you can replace that last commit by an update of manageiq-ui-components to 0.0.6.

@skateman
Copy link
Member Author

@himdel yup, and also the pf version to 3.9.0 :)

@skateman skateman changed the title [WIP] Patternfly upgraded to v3.8.1 [WIP] Patternfly upgraded to v3.9.0 Aug 29, 2016
@skateman skateman changed the title [WIP] Patternfly upgraded to v3.9.0 Patternfly upgraded to v3.9.0 Aug 29, 2016
@himdel
Copy link
Contributor

himdel commented Aug 29, 2016

Found one more issue (maybe pf-3.9?) .. hovering over the active menu item (secondary or tertiary) shows a black top border

active-menu-bug

(Added to the list)

@skateman
Copy link
Member Author

skateman commented Aug 29, 2016

This also happens on the patternfly 3.9.0 test pages, cc @jeff-phillips-18 patternfly/patternfly#442

@skateman
Copy link
Member Author

@himdel
Copy link
Contributor

himdel commented Aug 29, 2016

@skateman nice, that's probably everything then..

Should I merge, or do we wait for 3.10 (2 days I guess?)?

EDIT: guessing we don't want to wait, it's a minor issue

@skateman
Copy link
Member Author

@epwinchell can you also check this? LGTM

@skateman skateman changed the title Patternfly upgraded to v3.9.0 [WIP] Patternfly upgraded to v3.9.0 Aug 29, 2016
skateman and others added 3 commits August 30, 2016 16:00
this is needed because patternfly 3.8 changes the selectpicker structure..

Previously it would hide the <select> using .bs-select-hidden, and append the new button right after.
Now, it moves the <select> inside that button, and doesn't add any extra class.

Also, elem[0][1] is painful to read, use descriptive selectors and variables.

(cherry picked from commit 51bafeb246602e98ac6c3effec1b184bb9471b75)
we keep the toolbar wrapping margin fix, and remove the resulting double margin by forcing toolbar-pf-actions' bottom margin to 0

.. and remove duplicate css introduced in ManageIQ#9753
@skateman
Copy link
Member Author

@epwinchell @himdel pin/unpin buttons fixed

@skateman skateman changed the title [WIP] Patternfly upgraded to v3.9.0 Patternfly upgraded to v3.9.0 Aug 30, 2016
@skateman
Copy link
Member Author

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Aug 30, 2016
@serenamarie125
Copy link

FYI, we want the pin icon to be used. Currently PatternFly has the arrows rather than the pin - so we should change that once the upgrade is complete

@jeff-phillips-18
Copy link
Member

To change to pins for the collapse button in the header you can override with:

.secondary-collapse-toggle-pf {
&:before {
content: '\f08d';
}
&.collapsed {
&:before {
-ms-transform: rotate(45deg);
-webkit-transform: rotate(45deg);
transform: rotate(45deg);
content: '\f08d';
display: inline-block;
}
}
}

@miq-bot
Copy link
Member

miq-bot commented Sep 1, 2016

Checked commits skateman/manageiq@ad4633a~...b82024c with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 2 offenses detected

app/views/layouts/_content.html.haml

  • ⚠️ - Line 18 - Line is too long. [161/160]
  • ⚠️ - Line 3 - Line is too long. [161/160]

@jeff-phillips-18
Copy link
Member

👍

@himdel
Copy link
Contributor

himdel commented Sep 1, 2016

Not seeing any more UI breakage, merging..

@skateman We should however upgrade to 3.10 soon because of that active menu item bug :)

@himdel himdel merged commit 23af549 into ManageIQ:master Sep 1, 2016
@himdel himdel deleted the pf-update-3.8.1 branch September 1, 2016 12:57
@himdel himdel restored the pf-update-3.8.1 branch September 1, 2016 12:57
@himdel himdel added this to the Sprint 46 Ending Sep 12, 2016 milestone Sep 1, 2016
@himdel himdel added the darga/no label Sep 1, 2016
@himdel
Copy link
Contributor

himdel commented Sep 1, 2016

@jeff-phillips-18 @serenamarie125 any idea when that pin change will get into patternfly?

@skateman skateman deleted the pf-update-3.8.1 branch September 1, 2016 12:59
@serenamarie125
Copy link

@himdel I will talk to Leslie this week. There was some reluctance to taking this, but based on the user feedback that I've been getting from CloudForms customers, I'm still suggesting we go with it. I'll get back to you

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.

None yet

7 participants