Skip to content

Conversation

Gold872
Copy link
Member

@Gold872 Gold872 commented Sep 18, 2024

Instead of having a switch that can display the incorrect laser state, it now uses toggle buttons and only displays the laser state of the gripper data received

image

@Levi-Lesches
Copy link
Member

Levi-Lesches commented Sep 18, 2024

We've tried this before, but had the issue that the UI isn't responsive to what you press, which makes people think the Dashboard is broken. Currently, the metrics sidebar shows what the rover is actually doing while the switch we have now shows what you're trying to do. Not saying that's the best answer but an unresponsive UI isn't great either

Also, +1 for including a pic in the PR, -1 for keeping your auto-formatter on! It's tempting I know, but it distracts from the actual changes of the PR, and is best done separately where it can be insta-approved instead

@Gold872
Copy link
Member Author

Gold872 commented Sep 18, 2024

One other option I thought of (which I did in my other dashboard) is display what the user selected but have a check or X icon next to it to indicate whether or not the data the rover has is the one being displayed

Also where did I screw up formatting?

@Gold872
Copy link
Member Author

Gold872 commented Sep 18, 2024

Modified it so there's now an indicator displaying whether or not the states match (and there's a tooltip too)

image

image

Levi-Lesches
Levi-Lesches previously approved these changes Sep 19, 2024
Copy link
Member

@Levi-Lesches Levi-Lesches left a comment

Choose a reason for hiding this comment

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

Great, thanks. I just changed the warning into a sync icon since it's not "bad", per-se. And I updated the tooltip to be a bit simpler.

Also where did I screw up formatting?

Eg, some parts of the IK math. It's not bad formatting, it's an improvement, just makes reviews a bit more distracting is all.

Levi-Lesches
Levi-Lesches previously approved these changes Sep 19, 2024
@Gold872 Gold872 merged commit 7e1e502 into main Sep 19, 2024
1 check passed
@Gold872 Gold872 deleted the laser-light-display-fix branch September 19, 2024 19:30
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.

2 participants