-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Storage redesign #19472
Storage redesign #19472
Conversation
2c0f895
to
12fbfa4
Compare
12fbfa4
to
77dfe3b
Compare
77dfe3b
to
5ba71c6
Compare
5ba71c6
to
9c18f61
Compare
9c18f61
to
938efe8
Compare
938efe8
to
9a01886
Compare
9a01886
to
95084ef
Compare
95084ef
to
24d0cc7
Compare
24d0cc7
to
14dbdae
Compare
14dbdae
to
ff69b87
Compare
Issues:
|
a177612
to
262c829
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.
phew, I walked through all the pkg/storaged/ changes, I'll do a separate review for the tests. First of all, I like the file/class/function structure -- it's very symmetric, which helps with reviewing. I ignored (almost) all of JS code style issues, the dialogs, and the Reacty/design bits, and mostly looked for odd/unexplained/magic bits.
I realize that this bitrots fast, so I'm happy for most/all of my comments to be addressed in a follow-up. I'm just asking to not land this in today's release, so that we have two weeks for follow-ups.
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.
Tests are fairly straightforward, but they may have spotted a bug. Thanks for the careful pixel coverage!
a200b40
to
229f499
Compare
Coverage notes:
|
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.
Cheers! I reviewed the fixups since my last review, and updated the review threads. There are no more blockers from my side, and tests are green (f39 appears red because of a github flake with the API, but it did succeed).
I leave the final sign-off to @garrett , for the remaining discussions about icons and such.
Looking forward to seeing this land! 🏁
a6daebb
to
034884d
Compare
I can't navigate by keyboard.
(I can focus a row and also focus the menu for each row, but I can't activate them.) This is important for accessibility. |
Icons don't have enough contrast in dark mode. (They need to be lighter.) Edit: Fixed! |
5208c52
to
e830264
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.
I additionally reviewed the kezboard navigation, and while it is "ugh we need to do that?", it technically looks fine to me. Thanks!
e830264
to
aafdd31
Compare
Co-authored-by: Garrett LeSage <[email protected]>
aafdd31
to
a5f87a9
Compare
Amazing work! Thanks everyone! |
Overview demo: https://www.youtube.com/watch?v=aNS_4BejTDs
Mobile demo: https://www.youtube.com/watch?v=Gq7gBY-CciY
Polish demos:
Disabled menu items: https://www.youtube.com/watch?v=AQ9VU-FnzXs
Dangerous actions into menus: https://www.youtube.com/watch?v=RD9kMnKZ420
No "/dev/" prefix in the table: https://www.youtube.com/watch?v=FlD9OIgdxRM
Icons: https://www.youtube.com/watch?v=eLT_s5Y7Vaw
Different icons, "Size" header, indentation: https://www.youtube.com/watch?v=QrL3Pk9ZYvg
Raid stopping: https://youtu.be/U5gEZgY3N4k
Cards right side up: https://youtu.be/diSU7Hwt20Q
Fewer cards, no arrows: https://www.youtube.com/watch?v=lvyyiIh3aIg
Kezboard nav: https://www.youtube.com/watch?v=2lAcG-gouWw
Old demos:
https://youtu.be/aHir7PE9i2s
https://youtu.be/Mt7CkeDC9mQ
https://youtu.be/YwKPY3xjdPk
https://www.youtube.com/watch?v=zJHDXO3wVa4
Release notes
The Storage page has been redesigned with the goal of making more things visible more of the time. The overview page now shows all storage objects in a table and allows almost all operations to be performed on them right from the table.
Clicking on a row navigates to a page dedicated to the object in that row, with more information and maybe a few more actions.
We will continue to improve the Storage pages, by adding filtering options to the big table and working on the actions and dialogs.