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.

Sign up for GitHub

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

Jump to bottom

PT#139486261 - Update angular patternfly #804

Merged
merged 13 commits into from
Jun 7, 2017
Merged

PT#139486261 - Update angular patternfly #804

merged 13 commits into from
Jun 7, 2017

Conversation

AllenBW
Copy link
Member

@AllenBW AllenBW commented Jun 1, 2017 •
edited
Loading

https://www.pivotaltracker.com/story/show/139486261

Major work performed

  • Switch all attribute directives to component forms, i.e <div pf-list-view> to <pf-list-view>
  • Find and replace usage of pf-datepicker which was also removed in angular-patternfly 4
  • Upgrade application code to use lodash 4
  • rework pf-select
  • replace customScope with $ctrl.customScope
  • correct form implementations, they're off in places
  • Audit usage of each converted component, in a short time of testing found a few issues with pf-list-view customScope not working as expected

GIF of exploring each of the changes (I tried to retire something that isn't retierable, excuse this thing that looks like a bug, not related to the pf4 jump)

ang-pf4

AllenBW and others added 6 commits June 1, 2017 10:09
@AllenBW
Upgrade lodash to 4.x, all tests pass
f48a4be 
Replaces depreciated function sum with sumBy 
Replaces depreciated function pluck with map + values
@AllenBW
To componets from directive refactor
773c7c0 
Ignores pf-select, pf-datepicker, for later commits
@AllenBW
Refactor pf-select directives to components
55e59d9
@AllenBW
Refactor usage-graphs spec to select correct element
3c86e81
@AllenBW
Refactors language-switcher to support pf-select component
b349c15 
Original functionality maintained
@AllenBW
Removed duplicate component select-dropdown
5a7f222 
Ends up we're gonna just use pf-select, this has no purpose
@AllenBW AllenBW added this to the Sprint 62 Ending Jun 5, 2017 milestone Jun 1, 2017
AllenBW added 5 commits June 1, 2017 10:30
@AllenBW
Remove templates states and components
74eab19 
Dorment code pruned in lieu of refactoring it to utilize ang-pf 4.x
@AllenBW
Refactors pf-datepicker to use uib-datepicker
bad551f 
The styling isn't an exact match here, something to do later
@AllenBW
Restore order and filtering functionality to catalog explorer
43e1b9c 
Ends up we had a timey whimey issue declaring the filter and backfilling with the values when they are avalaible is the solution in this case
@AllenBW
Ensure selection count is hidden for catalogs shown for services
5790d85 
Otherwise we get some random text in the filter results bar
@AllenBW
Ensure my orders selection results are reported correctly
07347d9
@AllenBW AllenBW mentioned this pull request Jun 2, 2017
Closed
@AllenBW
Copy link
Member Author

AllenBW commented Jun 2, 2017

Ok mildly bad news, kinda blocked by a bug atm, documented here working on work arounds...

@AllenBW
Copy link
Member Author

AllenBW commented Jun 2, 2017

Never mind the above, we're good patternfly/angular-patternfly#479

@chriskacerguis chriskacerguis self-assigned this Jun 2, 2017
@AllenBW
Ensure all pf-form-group elements contain necessary attributes
184ade7 
pf-input-class and pf-label-class are now required for form groups to show up correctly
@miq-bot
Copy link
Member

miq-bot commented Jun 5, 2017

Checked commits AllenBW/manageiq-ui-service@f48a4be~...184ade7 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@AllenBW AllenBW changed the title [WIP] PT#139486261 - Update angular patternfly PT#139486261 - Update angular patternfly Jun 5, 2017
@AllenBW AllenBW removed the wip label Jun 5, 2017
@AllenBW
Copy link
Member Author

AllenBW commented Jun 5, 2017

I'm pulling off wip on this... reasonably satisfied it'll go the distance... but will likely require a little bit of merge work once #792 makes it in

@chriskacerguis chriskacerguis requested a review from chalettu June 5, 2017 14:03
@chriskacerguis
Copy link
Contributor

@himdel @h-kataria would love some feedback, if you have a minute.

/cc @dclarizio @chessbyte @Fryguy

@chriskacerguis
Copy link
Contributor

@serenamarie125 for your review

@chriskacerguis
Copy link
Contributor

@Loicavenel FYSA

@AllenBW
Copy link
Member Author

AllenBW commented Jun 6, 2017

Related to this pr... I made this over here... https://github.com/patternfly/angular-patternfly/pull/481/files

chalettu
chalettu approved these changes Jun 6, 2017
View reviewed changes
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.

Changes look good to me! Great work

@serenamarie125
Copy link

From what i see, this looks great visually - thanks for doing this :)
I'm cc'ing @jeff-phillips-18 so he can take a look as well

serenamarie125
serenamarie125 approved these changes Jun 6, 2017
View reviewed changes
Copy link

@serenamarie125 serenamarie125 left a comment

Choose a reason for hiding this comment

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

LGTM! Can you give Jeff a few hours to take a look as well?

@chriskacerguis
Copy link
Contributor

@serenamarie125 not a problem! :)

@chriskacerguis chriskacerguis merged commit b08039f into ManageIQ:master Jun 7, 2017
@AllenBW AllenBW deleted the enhancement/#139486261-update-angular-patternfly branch June 7, 2017 12:48
@AllenBW AllenBW mentioned this pull request Jul 7, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalettu chalettu chalettu approved these changes

@chriskacerguis chriskacerguis chriskacerguis approved these changes

@jeff-phillips-18 jeff-phillips-18 jeff-phillips-18 approved these changes

@serenamarie125 serenamarie125 serenamarie125 approved these changes

Assignees

@chriskacerguis chriskacerguis

Labels
enhancement ux/approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AllenBW @miq-bot @chriskacerguis @serenamarie125 @chalettu @jeff-phillips-18