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

Remove power down and restart buttons #5307

Merged
merged 3 commits into from
Aug 27, 2024
Merged

Conversation

jonahkagan
Copy link
Collaborator

@jonahkagan jonahkagan commented Aug 26, 2024

Overview

Closes #5214

  • Remove power down button from all apps except VxScan. Every other app has a physical power button, so this on-screen button is unnecessary.
  • Remove restart button from error boundary screen. The error boundary is mostly triggered by server crashes, in which case the restart button doesn't actually work (since it goes through the server).

Demo Video or Screenshot

Testing Plan

Updated automated tests

Checklist

  • I have added logging where appropriate to any new user actions, system updates such as file reads or storage writes, or errors introduced.
  • I have added a screenshot and/or video to this PR to demo the change
  • I have added the "user_facing_change" label to this PR to automate an announcement in #machine-product-updates

@jonahkagan jonahkagan requested review from a team and eventualbuddha and removed request for a team August 26, 2024 22:20
Copy link
Collaborator

@adghayes adghayes left a comment

Choose a reason for hiding this comment

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

In the app.tsx pages, we now have two identical error boundaries, one inside the ApiProvider and one outside. I recommend removing the ones inside the ApiProviders. I added those purely to have an error boundary with access to the API, for the purpose of rebooting, but now that's no longer necessary.

@jonahkagan
Copy link
Collaborator Author

In the app.tsx pages, we now have two identical error boundaries, one inside the ApiProvider and one outside. I recommend removing the ones inside the ApiProviders. I added those purely to have an error boundary with access to the API, for the purpose of rebooting, but now that's no longer necessary.

Thanks for this context. I was wondering why there were two error boundaries. Will update

Every other app has a physical power button, so this on-screen button is
unnecessary.
The error boundary is mostly triggered by server crashes, in which case
the restart button doesn't actually work (since it goes through the
server).
Now that the error boundary doesn't use the API at all, we don't need an
extra one inside the API provider
@jonahkagan jonahkagan force-pushed the jonah/remove-power-down-button branch from 765f0ab to 584cf30 Compare August 27, 2024 15:26
@jonahkagan jonahkagan enabled auto-merge (squash) August 27, 2024 15:26
@jonahkagan jonahkagan enabled auto-merge (squash) August 27, 2024 15:26
@jonahkagan jonahkagan merged commit a660fd1 into main Aug 27, 2024
62 checks passed
@jonahkagan jonahkagan deleted the jonah/remove-power-down-button branch August 27, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants