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

[UBP] Pagination and UI fixes #11481

Merged
merged 1 commit into from
Jul 21, 2022
Merged

[UBP] Pagination and UI fixes #11481

merged 1 commit into from
Jul 21, 2022

Conversation

laushinka
Copy link
Contributor

@laushinka laushinka commented Jul 19, 2022

Description

Among other UI fixes, this PR:

Screenshot 2022-07-21 at 13 04 44

Related Issue(s)

Fixes #11483
Fixes #11285

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-lau-ubp-iterate-11285.3 because the annotations in the pull request description changed
(with .werft/ from main)

@roboquat roboquat added size/XL and removed size/L labels Jul 20, 2022
@laushinka laushinka force-pushed the lau/ubp-iterate-11285 branch 2 times, most recently from 0b1d4ca to c9530f3 Compare July 20, 2022 16:17
@roboquat roboquat added size/L and removed size/XL labels Jul 20, 2022
@roboquat roboquat added size/XL and removed size/L labels Jul 20, 2022
@laushinka laushinka marked this pull request as ready for review July 20, 2022 17:22
@laushinka laushinka requested a review from a team July 20, 2022 17:22
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jul 20, 2022
@laushinka
Copy link
Contributor Author

laushinka commented Jul 20, 2022

This PR does not yet:

  • show the user avatar for workspaces (will tackle in a separate PR)
  • show the real usage period (currently the usage API call only has attributionId as argument)

@laushinka laushinka requested a review from jldec July 20, 2022 17:34
@easyCZ
Copy link
Member

easyCZ commented Jul 21, 2022

/werft run with-preview

👍 started the job as gitpod-build-lau-ubp-iterate-11285.15
(with .werft/ from main)

Copy link
Member

@easyCZ easyCZ left a 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

Copy link
Contributor

@jldec jldec left a 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)

@laushinka
Copy link
Contributor Author

laushinka commented Jul 21, 2022

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

@jldec
Copy link
Contributor

jldec commented Jul 21, 2022

@gtsiolis - since you're working on followup design polish I had some suggestions:

  • remove extra heading above the list (takes up valuable real estate)
  • move existing description up
  • small polish on sidebar - heading bold, values not-bold,
  • "Total usage" instead of "Monthly usage" as discussed (and don't repeat the word "total" before credits)

Screenshot 2022-07-21 at 11 45 08

@gtsiolis
Copy link
Contributor

gtsiolis commented Jul 21, 2022

Woohoo! UBP is getting real. ✨

De rien, @laushinka!

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.

A B C
Usage Usage-1 Usage-2

@jldec
Copy link
Contributor

jldec commented Jul 21, 2022

@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

@gtsiolis
Copy link
Contributor

gtsiolis commented Jul 21, 2022

I don't see value in the extra heading - It steals vertical space from the list

@jldec Fair point. Up for removing it and see how it feels. 🤝

I find the bold black summary element in the sidebar a bit distracting so prefer option C for that reason.

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

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".)

@jldec This is already changed in #11526.

@gtsiolis
Copy link
Contributor

gtsiolis commented Jul 21, 2022

I find the bold black summary element in the sidebar a bit distracting ...

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:

BEFORE AFTER What's on production
Sidebar-1 Sidebar Sidebar3

From #11481 (comment):

I may also attempt to create a version that's not so bold using black background, but a more grayish background. 🤝

BEFORE AFTER What's on production
Usage Usage-1 Screenshot 2022-07-21 at 6 31 03 PM (2)

@jldec
Copy link
Contributor

jldec commented Jul 21, 2022

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.

@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/XL team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pagination or infinite scroll support [UBP] Team usage view - Iterate based on designs
5 participants