-
Notifications
You must be signed in to change notification settings - Fork 91
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
Conversation
@serenamarie125 @Loicavenel for your review! |
@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? |
|
@serenamarie125 here is the updated screenshot . For your review. |
This pull request is not mergeable. Please rebase and repush. |
templateUrl, | ||
controller: Controller, | ||
controllerAs: 'vm', | ||
title: __('Service Template Details'), |
There was a problem hiding this comment.
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 🎋
There was a problem hiding this 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 🌮
Cool @chalettu ... any chance that Reorder button can be right aligned on the list view ? |
There was a problem hiding this 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!
@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 |
@serenamarie125 , please let me know if my answer from yesterday makes sense and if we can proceed. |
This pull request is not mergeable. Please rebase and repush. |
@serenamarie125 does @chalettu make sense, or should I setup a meeting to chat about that? /cc @Loicavenel |
@serenamarie125 does @chalettu make sense, or should I setup a meeting to chat about that? /cc @Loicavenel |
@chalettu one question on my side, does it use the previous valued entered (what we want) or the reset to default? thanks |
@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 ? |
@Loicavenel This uses the previous value entered for the service you are trying to duplicate |
Champagne!! |
@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" |
|
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 |
@serenamarie125 , did you have any more thoughts on this PR after reviewing the last screenshot and comments I submitted? |
@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? |
@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.. |
@miq-bot remove_label ux/review |
@miq-bot add_label ux/approved |
There was a problem hiding this 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.
@AllenBW had previous approved this (but, it was dismissed due to a UX change), so going to merge. |
@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