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

Quota warnings #59

Merged
merged 5 commits into from
Sep 6, 2017
Merged

Conversation

ncameronbritt
Copy link
Contributor

- Toasts and notification drawer messages should include a kebab menu with options to:
- View Quotas
- Don't Show Me Again
- Close notification
Copy link
Member

Choose a reason for hiding this comment

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

@ncameronbritt Should this last option actually be "Clear?" That's the action on the right side of the other rows and also the syntax used for the bottom action link, "Clear All."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense to me. Would that be the same for toasts too?

Cameron Britt added 2 commits August 28, 2017 16:40
…n-design into quota-warnings

* 'master' of https://github.com/openshift/openshift-origin-design:
  Updating images and clarifying details based on comments
  Adding Deployment Details page to document new features around Environment Variables
@beanh66
Copy link
Member

beanh66 commented Aug 30, 2017

@ncameronbritt as I mentioned, it might be beneficial to add a mockup to the "On Add to Project" section to illustrate what we want those messages to look like in the Results step, but it sounds like this is part of a follow-up story so I think this looks good!

Copy link
Member

@beanh66 beanh66 left a comment

Choose a reason for hiding this comment

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

LGTM

@benjaminapetersen
Copy link

Quick Q! Was just talking with @spadgett about a few more drawer items & this PR came up. What happens if someone misses the toast ( perhaps looks away for a second) & the quota notification is also pushed below the fold? Its pretty important, arguably more important than other notifications.

Would we want to consider a "pinned" notification concept? Somehow ensuring certain notifications stay above the fold:

  • Perhaps a very small subset of notifications (like quota) that actually appear above the project heading (with special visual treatment)
  • Or a different notificationGroup (own heading) for these items
  • Or in our sorting function, we sort by timestamp (as current) but certain notifications (like quota) get a pass & bubble top due to significance.

Def interested in your thoughts on this.

@ncameronbritt
Copy link
Contributor Author

@benjaminapetersen Good question. I suppose you could ask the same question about any message we toast. Seems like the feedback we're getting from customers is that they really only want to see notifications in the drawer for things the are actually problems. So perhaps the way to go is to be even more selective with the kinds of things that make it to the notification drawer, so that important messages are less likely to get pushed down. Other thoughts, @openshift/team-ux-review ?

@serenamarie125
Copy link
Contributor

if it's more important than others, i'd assume it wouldn't be informational :D

I think that going forward we may want to think about enhancements to the drawer at a higher level, and i'd guess that sort, filter and even pinning may be part of those.

Alternatively associated with this specific issue, we COULD have a separate accordion for quota warnings, if we feel that they are that important. The PF component supports this already. @ncameronbritt @beanh66 @benjaminapetersen @jeff-phillips-18 what do you think about that?

@jeff-phillips-18
Copy link
Member

@benjaminapetersen Toast notifications can be set to be persistent, meaning they do not auto-close the user must specifically dismiss the toast. Is that helpful in your scenario?

@serenamarie125 You can have as many categories of notifications as you want, seems like it could be useful.

@spadgett
Copy link
Member

spadgett commented Sep 1, 2017

I think that going forward we may want to think about enhancements to the drawer at a higher level, and i'd guess that sort, filter and even pinning may be part of those.

I really want pinning :)

Alternatively associated with this specific issue, we COULD have a separate accordion for quota warnings, if we feel that they are that important.

It could be more than just quota. One category for events and another for messages/alerts?

@beanh66
Copy link
Member

beanh66 commented Sep 1, 2017

I agree @spadgett @serenamarie125, we could easily use the accordion to delineate sections within the drawer since that functionality is already supported, but I can see a use for pinning messages in the future. Seems similar to email with starring or marking as unread.

@serenamarie125
Copy link
Contributor

@spadgett we can add a design story around pinning, but I don't think we would want to push that into 3.7. I'm not sure i fully understand the use cases around it and would like some validation with users before doing something like that. Would adding a design story around that be sufficient for now?

And Yes, multiple categories works. If we separate, we would then want to make sure that the View All Events link is in the appropriate accordion. I think that splitting events & messages makes a lot of sense. I'm not sure that as a user i would expect to see messages & alerts in the same "drawer". thoughts @beanh66 @ncameronbritt ?

@spadgett is the latter something you would consider for 3.7?

@ncameronbritt
Copy link
Contributor Author

I have no problem with splitting up events and messages into accordion sections. It could help, but I don't think it completely solves the problem of notifications being pushed below the fold though. If there are enough messages, some are going to get pushed down regardless of whether they're split into separate categories/accordions or not.

@serenamarie125 you touched on the question of whether these quota notifications should be warnings or informational. I'm happy to discuss or revisit that question, but the issue Ben brought up would apply regardless of the severity of the notification.

@ncameronbritt
Copy link
Contributor Author

Savvy users might understand the distinction between events and other types of messages, but I don't know how useful that distinction is. I would imagine that users would want to know that that there's a problem regardless whether that problem is reported as an event or by some other means.

@benjaminapetersen
Copy link

things the are actually problems

This is a bit tricky as we have Normal and Warning events, but Warnings can resolve themselves (so is it actually a problem?). Just tacking that onto our discussion about the nuances of notifications.

The next version of the Events API (issue link here) and google doc for design. As it looks right now there will be both event.severity with values like TakeALookTomorrow, InvestigateInstantly as well as event.reason (which looks in flux). Most of the API looks in flux yet, as it has changed since last week, but in the long run it will give us much better control over what we do with events in the drawer.

@spadgett
Copy link
Member

spadgett commented Sep 1, 2017

we can add a design story around pinning, but I don't think we would want to push that into 3.7. I'm not sure i fully understand the use cases around it and would like some validation with users before doing something like that. Would adding a design story around that be sufficient for now?

Yeah, I wasn't suggesting we add pinning in 3.7. Just a personal want after using the drawer.

To be clear, when I say "pinning" I mean I'd like to leave the drawer open without it covering content on the page. Then I can watch events. Right now it covers too much basic information on the page to leave open all the time.

@spadgett is the latter something you would consider for 3.7?

I'm hesitant to sign up for additional things for 3.7 given that we only have one more sprint and several committed cards left. Right now since the drawer is primarily showing events (and maybe a few quota warnings), I don't see it as urgent for 3.7. It's more of a problem later if we add more things to the drawer.

@serenamarie125 Sound OK?

@benjaminapetersen
Copy link

benjaminapetersen commented Sep 1, 2017

@spadgett i take your comment on "pinning" = set the drawer into the page as if it was a sidebar on the right.

@serenamarie125
Copy link
Contributor

makes sense @spadgett

and oh ... @benjaminapetersen i didn't understand that as the concept of pinning, that makes sense to me now :D

@serenamarie125
Copy link
Contributor

So for the purpose of the initial design here, is the current PR sufficient? Or do we want some changes

@benjaminapetersen
Copy link

@serenamarie125 yup wanted to clarify as I think I used the word "pin" to mean something slightly different in my first comment above.

Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

I agree with all the suggestions, although some could be extremely difficult to implement. We might need to do a basic implementation using the drawer and custom messages first and then evaluate which of the other suggestions are practical / possible.

- The total percentage requested (to avoid confusion with units) and max plus units.
- Information/link to help the user take action if action is needed.
- Link to quota page.
- Since the quota doesn't represent actual utilization, the verb "requested" should be used in the message rather than "used".
Copy link
Member

Choose a reason for hiding this comment

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

The term "request" has a very specific meaning in Kubernetes and OpenShift, so I wouldn't use it here. Compute resources have both limit and request quotas and if you use the term "requested" for limit it is going to be really confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what the correct verb would be here since the quota doesn't actually reflect utilization. Open to suggestions here!


### At quota for compute-resource
- User should be alerted via an informational message.
- If the user takes an action that brings her to the quota, she should see a toast notification.
Copy link
Member

Choose a reason for hiding this comment

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

This is really hard to anticipate. I'm not sure if we'll be able to know always.


## On Add to Project
- If Add to Project fails due to insufficient quota, the Results step of the wizard should give information about the quotas and suggestions to address the issue (e.g. upgrade to Pro, Add More Storage, etc)
- It would be useful to know before attempting to provision (and entering config information) if add to project is going to fail due to insufficient quota.
Copy link
Member

Choose a reason for hiding this comment

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

With service catalog, this is going to be really difficult to know unfortunately.

@ncameronbritt
Copy link
Contributor Author

Thanks for the review @spadgett. I'll either label some of the things you had concerns about as "Future" or just take them out.

Copy link
Contributor

@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.

@serenamarie125 serenamarie125 merged commit f741ed9 into openshift:master Sep 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants