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

[Finishes #146028835] Allowed users to duplicate services #920

Merged
merged 6 commits into from
Sep 26, 2017
Merged

[Finishes #146028835] Allowed users to duplicate services #920

merged 6 commits into from
Sep 26, 2017

Conversation

chalettu
Copy link
Contributor

@chalettu chalettu commented Sep 8, 2017

@miq-bot add_label enhancement
@miq-bot add_label fine/no
@miq-bot add_label catalogs / dialogs
@miq-bot add_label ux/review

See #937

@chalettu
Copy link
Contributor Author

chalettu commented Sep 8, 2017

editdialog
Here is a screenshot of what duplicating an existing service from our Order explorer looks like. From a UX perspective it wasn't much of a change.

@chriskacerguis chriskacerguis self-assigned this Sep 8, 2017
@chriskacerguis chriskacerguis added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 8, 2017
@chriskacerguis
Copy link
Contributor

@serenamarie125 @Loicavenel for your review!

@Loicavenel
Copy link

@chalettu Video is confusing, you are taking are clicking on Order but ending up in the Service, I don't see how you select a service in the order?

@chalettu
Copy link
Contributor Author

new_reorder
Updated Screenshot for reordering of a service. @Loicavenel we tried to push the button further right but can't due to limitations of it being an expanded content row of the Order above it. There is a certain amount of padding in the patternfly component that we can't override.

@chalettu
Copy link
Contributor Author

@serenamarie125 here is the updated screenshot . For your review.

@miq-bot
Copy link
Member

miq-bot commented Sep 12, 2017

This pull request is not mergeable. Please rebase and repush.

templateUrl,
controller: Controller,
controllerAs: 'vm',
title: __('Service Template Details'),
Copy link
Member

Choose a reason for hiding this comment

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

Thinking this title should probably change 🎋

Copy link
Member

@AllenBW AllenBW left a comment

Choose a reason for hiding this comment

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

Small change, a change to the title to reflect the different state, that aside LG2M!! <3 🌮

@serenamarie125
Copy link

Cool @chalettu ... any chance that Reorder button can be right aligned on the list view ?

AllenBW
AllenBW previously approved these changes Sep 13, 2017
Copy link
Member

@AllenBW AllenBW left a comment

Choose a reason for hiding this comment

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

Like it, love it, can't get enough of it!

@chalettu
Copy link
Contributor Author

@serenamarie125 , that button is pushed as far right as it can get. It is an issue with the patternfly listview and having listviews in the expanded contents. We looked into it and its a matter of all the padding that gets added into the patternfly list view component

@chalettu
Copy link
Contributor Author

@serenamarie125 , please let me know if my answer from yesterday makes sense and if we can proceed.

@miq-bot
Copy link
Member

miq-bot commented Sep 14, 2017

This pull request is not mergeable. Please rebase and repush.

@chriskacerguis
Copy link
Contributor

@serenamarie125 does @chalettu make sense, or should I setup a meeting to chat about that?

/cc @Loicavenel

@chriskacerguis
Copy link
Contributor

@serenamarie125 does @chalettu make sense, or should I setup a meeting to chat about that?

/cc @Loicavenel

@Loicavenel
Copy link

@chalettu one question on my side, does it use the previous valued entered (what we want) or the reset to default? thanks

@chalettu
Copy link
Contributor Author

@serenamarie125 the screenshot is using our dialog component on a page. In the context of your last comment, are you intending to say the difference between being a modal vs being part of the page? This particular screenshot is part of a larger page and is not a modal. That being said, are we fine with the button placement ?

@chalettu
Copy link
Contributor Author

@Loicavenel This uses the previous value entered for the service you are trying to duplicate

@Loicavenel
Copy link

Champagne!!

@serenamarie125
Copy link

@chalettu If it's not a modal, what does the Navigation look like? Can you increase the size of the image so we can see that, as well as show where the user is left after they click on "Add to Shopping Cart"

@chalettu
Copy link
Contributor Author

add_to_cart
@serenamarie125 here is a bigger screen shot of the actual add to cart process.

@miq-bot
Copy link
Member

miq-bot commented Sep 22, 2017

Checked commits https://github.com/chalettu/manageiq-ui-service/compare/85e0106474e2cc0596cf0f18dc102c66d6f099af~...aa0c7027db00cfcfb64e5530e79b6a0b30f59b12 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

@chalettu
Copy link
Contributor Author

@serenamarie125 , did you have any more thoughts on this PR after reviewing the last screenshot and comments I submitted?

@serenamarie125
Copy link

serenamarie125 commented Sep 26, 2017

@chalettu just to make sure i understand, the user goes to "My Orders", finds a request to Reorder, and then is moved to the Service Catalog area? I think that's a bit funky, and that's why I was suggesting that it be put into an overlay panel. @Loicavenel what are your thoughts here, are you ok with the current flow?

@Loicavenel
Copy link

@serenamarie125 moving to the service Catalog does not make any sense. the way it was implemented in the second animation is perfect for me. You go to My oder, you find an order, you expend it, then you select the service you want to re-order... click re-oder, show the dialog right there with overlay..

@serenamarie125
Copy link

@miq-bot remove_label ux/review

@serenamarie125
Copy link

@miq-bot add_label ux/approved

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.

After talking with @Loicavenel, we agreed that this PR should be merged. I've opened a new issue around the fact that the ordering flow should be moved to an overlay panel.

@chriskacerguis
Copy link
Contributor

@AllenBW had previous approved this (but, it was dismissed due to a UX change), so going to merge.

@chriskacerguis chriskacerguis merged commit f091980 into ManageIQ:master Sep 26, 2017
@chriskacerguis chriskacerguis modified the milestones: Sprint 69 Ending Sep 18, 2017, Sprint 70 Ending Oct 2, 2017 Sep 26, 2017
@chriskacerguis
Copy link
Contributor

@chalettu chalettu deleted the duplicate-order branch November 21, 2017 19:28
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