Skip to content

Comments

UI Post-Refactor Revisions [AARD-2002]#1268

Merged
PepperLola merged 29 commits intodevfrom
aries/2002/fix-ui-appearance
Aug 23, 2025
Merged

UI Post-Refactor Revisions [AARD-2002]#1268
PepperLola merged 29 commits intodevfrom
aries/2002/fix-ui-appearance

Conversation

@itspvty
Copy link
Contributor

@itspvty itspvty commented Aug 11, 2025

Task

AARD-2002

This task involved collecting bugs relevant to the UI refactor from you guys and writing fixes for each one.

Task list:

  • Toasts have a "findDOMNode is deprecated" warning in devtools
  • The logo on the sidebar is draggable
  • Panels are too tall (even with no content)
  • The developer tool "No mira field loaded" red color is ugly, as well as some other red texts
  • You can open more than one unique panel
  • Panels on top of each other don't have proper separation (maybe add box shadow)
  • The "spawn" window and the "configure assets" windows shouldn't be able to be open at the same time
  • The dropdown shadows are too subtle/weird
  • Can't drag panels if pressing their title
  • The robot right-click context window shows the text "A Robot" in black at the top
  • Sliders do not display numerical values
  • There isn't enough spacing between many buttons (ex, match mode config menu)
  • The drag mode indicator at the bottom left has dark text on a dark bg
  • The timer on the scoreboard is dark text on dark bg
  • Where present, "delete" and "edit" buttons are placed weirdly (a line after their respective element often)
  • Add padding to the match mode config panel items
  • Block opening config/spawn when one of the two is already open
  • Fix slider overflow
  • Add info content to empty dropdowns
  • Pressing plus on the configure asset panel doesn't open the spawn assets panel
  • No feedback for why the config panel doesn't open when the spawn asset panel is open
  • Panels can be too tall
  • The configure panel for inputs skips to a specific scheme
  • The APS import folder doesn't have any text when empty
  • Draggable region distinction
  • Fix analytics consent panel color
  • "Requires Browser Refresh" poorly styled; to tooltip
  • There are two scrollbars on tall panels
  • Blocked from closing the configure assets panel even with no configuration active
  • "Grab" cursor on draggable regions
  • Fix font sizes/logic throughout (titles/headers)
  • Fix spacing issues in more places
  • APS spinner alignment fix
  • Fix "submit" and "cancel" button overflow
  • Slider label/position uniformity
  • Fix input configure placements
  • Increase the maxHeight of all panels to 85vh
  • Make the panel title and button background the same color as the content
  • Keybind setters say "..." instead of "PRESS ANYTHING" when activated

Symptom

The UI refactor was a large code edit and brought various visual bugs with it. The UI was, in most cases, fully usable, but the styling wasn't production-ready.

Solution

Fixing individual issues over time by collecting purposeful data.

Verification

Look good? Yay! Look bad? Aw man :(


Before merging, ensure the following criteria are met:

  • All acceptance criteria outlined in the ticket are met.
  • Necessary test cases have been added and updated.
  • A feature toggle or safe disable path has been added (if applicable).
  • User-facing polish:
    • Ask: "Is this ready-looking?"
  • Cross-linking between Jira and GitHub:
    • PR links to the relevant Jira issue.
    • Jira ticket has a comment referencing this PR.

@itspvty itspvty self-assigned this Aug 11, 2025
@itspvty itspvty added the ui/ux Relating to user interface, or in general, user experience label Aug 11, 2025
@itspvty itspvty requested review from a team as code owners August 11, 2025 21:30
Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

Looks good overall but I noticed a couple things.

  • I think the match mode config panel items could use some padding
  • I'm not sure whether it makes more sense to close configure/spawn when the other is opened or to block the opening, because the user could be in the middle of configuring something and then opening spawn panel would cancel all their changes
  • When dragging a slider in the settings modal all the way to the left or right the handle gets cut off, and on the right side it overflows and causes there to be a horizontal scroll bar
  • I'm not sure how to address this, but I think the empty accordions in the spawn panel look a bit weird when expanded.

@rutmanz
Copy link
Member

rutmanz commented Aug 12, 2025

I'm not sure how to address this, but I think the empty accordions in the spawn panel look a bit weird when expanded.

Maybe just a message that is like "No Saved Assets" / "No Assets Found" rather than leaving it empty

@rutmanz
Copy link
Member

rutmanz commented Aug 12, 2025

I'm not sure whether it makes more sense to close configure/spawn when the other is opened or to block the opening, because the user could be in the middle of configuring something and then opening spawn panel would cancel all their changes

I think configure closing spawn is fine, but if configure is editing something (not just open) then spawn should show a warning

@itspvty itspvty requested a review from PepperLola August 12, 2025 17:41
@rutmanz
Copy link
Member

rutmanz commented Aug 12, 2025

  1. I think that configure assets should also not be openable when the initial config panel (right after spawning) is open because it lets you configure the same robot in two different places which I don't think makes sense
  2. The input screen doesn't look quite right still, and I also don't seem to be able to drag it anymore (use one of the controller input schemes)
image
  1. Reset orientation is kind of weird here, maybe just have one reset button?
image
  1. I don't think we always want to see values (particularly for scalars that don't actually have real-world meaning) on sliders. Also "Preferences" should be more of a header IMO
image

Copy link
Member

@rutmanz rutmanz left a comment

Choose a reason for hiding this comment

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

see above ^

@itspvty itspvty requested a review from rutmanz August 12, 2025 18:35
boxShadow: "0 4px 12px rgba(0,0,0,0.28), 0 2px 6px rgba(0,0,0,0.18)",
},
},
},
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about adding something like this? This blurs the background if you have a modal open which I think is better.

Suggested change
},
},
MuiBackdrop: {
styleOverrides: {
root: {
backgroundColor: "rgba(0, 0, 0, 0.5)",
backdropFilter: "blur(2px)",
},
},
},

@AlexD717 AlexD717 mentioned this pull request Aug 14, 2025
5 tasks
Copy link
Member

@rutmanz rutmanz left a comment

Choose a reason for hiding this comment

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

  • Pressing the plus button on the configure asset panel doesn't open the spawn assets panel anymore
  • There is no feedback for why the config panel doesn't open when the spawn asset panel is open. I think it makes more sense to make the newly-opened one replace the other, unless you're editing a configuration, at which point it should show a warning toast
  • Panels can still be too tall
Image
  • The configure panel for inputs seems to skip to a specific scheme instead of showing the list (and there are no breadcrums)
  • The APS import folder doesn't have any text when empty
  • If only parts of a panel are going to be draggable, there should be a visible distinction (maybe a handle like this)

@itspvty itspvty requested a review from rutmanz August 15, 2025 17:59
Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

The analytics consent panel text is a weird color and the buttons (at least the close button) need fixing.
Image

Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

  • I would like the APS spinner to be either the same height as the text (ideal) or centered vertically.
Image
  • The default height of the initial config panel makes the accept/cancel buttons overflow. Maybe a worthwhile change would be making the modal/panel content scrollable but keeping the buttons visible (and the title as well)
Image
  • If it's not too difficult it would be nice to have the slider labels match the other ones (in size + alignment/indentation); right now they look a bit out of place
Image
  • This toggle group should be centered horizontally like in the spawn assets panel, and the buttons shouldn't touch the bottom of the panel (latter would be fixed by always showing accept/cancel)
Image

Copy link
Member

@rutmanz rutmanz left a comment

Choose a reason for hiding this comment

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

Approving because you've addressed my previous review and everybody else has caught my remaining feedback

@AlexD717
Copy link
Member

How hard would it be for you to go through and increase the maxHeight of all panels to something closer to 85vh? I think that would be nicer.

Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

  • Can the panel title and button background be the same color as the content? I prefer the lighter background color between the two of them
  • Can we make the input buttons say "..." when you click them rather than "PRESS ANYTHING"?

Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

Can the sliders here be stacked vertically?

Image

The title and buttons have the old background in modals

Image

Can we switch the various configure panels to use tabs instead of the toggle group?

Image

There are two intake duration sliders

Image

@itspvty itspvty requested a review from PepperLola August 22, 2025 19:15
Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

The requested changes were not addressed.

- updated main hud with new buttons, made button group
- made modal colors consistent with panel
- switched to using tabs instead of toggle button groups
- fixed aps reload button disappearing on import panel reopen
@PepperLola PepperLola force-pushed the aries/2002/fix-ui-appearance branch from 00c3cfb to 399e93e Compare August 23, 2025 00:17
Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

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

Looks good

@PepperLola PepperLola merged commit 8e72603 into dev Aug 23, 2025
17 checks passed
@PepperLola PepperLola deleted the aries/2002/fix-ui-appearance branch August 23, 2025 00:23
@PepperLola PepperLola mentioned this pull request Aug 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui/ux Relating to user interface, or in general, user experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants