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

Delay based retirement #2282

Merged
merged 1 commit into from
Oct 10, 2017

Conversation

mzazrivec
Copy link
Contributor

@mzazrivec mzazrivec commented Oct 3, 2017

This enhancement allows for selecting service / vm / orchestration stack retirement date as
a time delay from now (implemented as an angular directive datetime-delay-picker).

Initial landing screen for service / vm / orchestration stack retirement, it shows standard datetime picker
widget allowing to select specific date & time for retirement:
delay-retirement-03

New functionality allows for entering the retirement date & time as a delay from now. After selecting
the option in the new dropdown menu, the following screen is shown:
delay-retirement-04

The screen contains inputs for months, weeks, days and hours from now, when the item in question
would be retired. Additionally, the screen also renders the supposed retirement date.

@dclarizio
Copy link

@serenamarie125 besides making the "spinner" controls a bit less wide, do you have any opinions on the looks of the interval selector?

@Loicavenel do you think we need minutes or is that too granular?

@serenamarie125
Copy link

IMO, the numbers should be aligned, and yes the fields should be less wide. Are you using the Bootstrap Spinner? FYI PatternFly doc on the pattern can be found here if you're interested: http://www.patternfly.org/pattern-library/forms-and-controls/data-input/#/bootstrap-touchspin

@Loicavenel
Copy link

@dclarizio I am a little confused here.. I suspect we are calling it from a VM or Service to extend retirement. Today, the option was to set a Date and we try to be more granular and offer a way to extend X days, minutes etc..
Can I still select the Date directly ? I suspect it is selectable from the first drop list? Should it be a Check a check selection between 2 items..
As first look, this seems very complicated, too many option can we aligned.. Like Days:hours:Minutes on the same ligne..

  • Minutes is really granular, I don't think people will push for 1H and 2 minutes.. they will push for 2 hours instead
  • Year looks crazy, we should remove it.

@serenamarie125 I think we need a quick mockup here..

@mzazrivec mzazrivec force-pushed the delay_based_retirement branch 5 times, most recently from f5d1528 to 89c334e Compare October 4, 2017 11:07
@mzazrivec
Copy link
Contributor Author

Another possible design:

delay-retirement-temp-02

@mzazrivec mzazrivec force-pushed the delay_based_retirement branch from 89c334e to 56e3efa Compare October 4, 2017 11:12
@Loicavenel
Copy link

LOOKS GREAT... @serenamarie125 your view?

@serenamarie125
Copy link

@mzazrivec can you provide before screenshot as well? What options are in the first drop down "Time Delay from Now"
@Loicavenel can you explain what the extra requirement here is?

@mzazrivec mzazrivec force-pushed the delay_based_retirement branch 2 times, most recently from 94f0798 to b60b0a9 Compare October 4, 2017 12:32
@Loicavenel
Copy link

@serenamarie125 the other option is to set the date, I don't know what is in the dropdown because before it was just the retirement date that was clickable..
I think we need a mockup with Radio button to select one or the other..
Also, the display of the date could be greatly improved if we do have a Calendar view circling the date.. As end user can have access to this function we should make it easy to read
@dclarizio did you sync up with Chris K on this one? Should we make a component?

@mzazrivec
Copy link
Contributor Author

@serenamarie125 I added some screenshots into the initial comment.

@mzazrivec mzazrivec force-pushed the delay_based_retirement branch 3 times, most recently from e208d19 to c985887 Compare October 4, 2017 13:58
@dclarizio
Copy link

@Loicavenel

@dclarizio did you sync up with Chris K on this one? Should we make a component?

This was an OPS UI RFE that we thought we could get into this release. If you'd like to expand it to both UIs, we can do that, but will add time. Let me know.

@mzazrivec mzazrivec force-pushed the delay_based_retirement branch from c985887 to 223d476 Compare October 5, 2017 11:00
@mzazrivec mzazrivec changed the title [WIP] Delay based retirement Delay based retirement Oct 5, 2017
@mzazrivec mzazrivec removed the wip label Oct 5, 2017
@mzazrivec mzazrivec force-pushed the delay_based_retirement branch 3 times, most recently from ae0e199 to 6f34511 Compare October 5, 2017 15:31
@dclarizio
Copy link

@Loicavenel bump, see comment above

@Loicavenel
Copy link

@dclarizio let's focus on OPS UI first and port it later to SUI

@dclarizio dclarizio self-assigned this Oct 5, 2017
@dclarizio
Copy link

@serenamarie125 can you re-review? Thx!

@serenamarie125
Copy link

@mzazrivec we are working on some mockups, not ignoring you. Hoping to get an answer to you tomorrow.

This enhancement allows for selecting retirement date as a time
delay from now (implemented as a new angular component:
datetime-delay-picker).
@mzazrivec mzazrivec force-pushed the delay_based_retirement branch from 6f34511 to d4a9003 Compare October 6, 2017 08:56
@miq-bot
Copy link
Member

miq-bot commented Oct 6, 2017

Checked commit mzazrivec@d4a9003 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@martinpovolny
Copy link
Member

Merging this. UX feedback will be handled in a separate PR. Sorry.

@martinpovolny martinpovolny merged commit 72349e4 into ManageIQ:master Oct 10, 2017
@martinpovolny martinpovolny added this to the Sprint 71 Ending Oct 16, 2017 milestone Oct 10, 2017
@mzazrivec mzazrivec deleted the delay_based_retirement branch February 22, 2018 12:27
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