-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[UBP] Pagination and UI fixes #11481
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
Conversation
started the job as gitpod-build-lau-ubp-iterate-11285.3 because the annotations in the pull request description changed |
46f3047
to
01bb7a1
Compare
0b1d4ca
to
c9530f3
Compare
c9530f3
to
b75714b
Compare
b21b73c
to
ade93b7
Compare
This PR does not yet:
|
/werft run with-preview 👍 started the job as gitpod-build-lau-ubp-iterate-11285.15 |
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.
Code-wise LGTM. Adding hold for @jldec as he's a reviewer too.
/hold
ade93b7
to
7e78440
Compare
7e78440
to
d607e67
Compare
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.
Thanks Laurie - this is a great step forward - let's merge this - it will be good to see real usage data. (leaving hold in case you are still working on TS errors)
Thanks! It's green now so I will unhold. Also thank you @gtsiolis for the quick UI sync-fixes 🔥 /unhold |
@gtsiolis - since you're working on followup design polish I had some suggestions:
|
Woohoo! UBP is getting real. ✨ Agree with all your points, @jldec, except for probably removing the section heading before updating the sidebar, which could confuse users of what exactly they are looking at. However, I'd be up for trying that and seeing in action if the heading is really helpful. To help demonstrate the issue here, let me add below some screenshot of how this page would look like a) with both a better sidebar and section heading, b) just the better sidebar, and c) no better sidebar and no heading. I've also opened a few follow-up issues to keep track of some UX issues in this page, see #11495, #11526, #11527, #11528, #11530, and #11532.
|
@gtsiolis I don't see value in the extra heading - It steals vertical space from the list (we have a lot of chrome already). I find the bold black summary element in the sidebar a bit distracting so prefer option C for that reason. Could we please label the total usage element "Total Usage" (not "Monthly Usage") and use "Credits" as the units next to the value (instead of "Total Credits".) cc: @laushinka |
@jldec Fair point. Up for removing it and see how it feels. 🤝
@jldec Thanks for the feedback! Let's go with C and change back if needed. I may also attempt to create a version that's not so bold using black background, but a more grayish background. 🤝
|
One more note regarding dropping the heavier dark-colored active elements in the left sidebar is that we would need to pay closer attention to the UX changes so that the visual balance remains good as originally seen in the design specs. Otherwise, everything will collapse together in dense-information pages like the usage page. The heavier dark-colored elements were also added to provide the necessary visual balance, but also information hierarchy required when using fewer borders in the user interface, see also #7932 (comment). This will make more sense once we introduce more drill-down pages for the usage page and use the left sidebar for switching between sub-pages, see relevant discussion (internal). To demonstrate this better:
From #11481 (comment):
|
Clarifying a little more - I think the black box draws attention (because of its size) in a way that makes it look like a heading (for the page) instead of a section element. |
Description
Among other UI fixes, this PR:
Related Issue(s)
Fixes #11483
Fixes #11285
How to test
Release Notes
Documentation
Werft options: