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

Add 'Reconnect DevTools' toolbar button #85

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

EdmondChuiHW
Copy link

@EdmondChuiHW EdmondChuiHW commented Jun 26, 2024

Summary

Add a simple toolbar button to

  1. communicate the UI is stale;
  2. allow reconnection.

screenshot of the new 'Reconnect DevTools' button

This is most useful when the "disconnected" dialog is dismissed, e.g. to inspect existing logs.

screenshot of the existing 'disconnected' dialog

Originally suggested by @rubennorte via T192737861

Test plan

Trigger a disconnection, e.g. terminate Metro, terminate the target app, etc.

Upstreaming plan

This PR is upstream-able in principle. However, a more sophisticated mechanism to check and display connection status could be Fusebox-specific in the future, e.g. a ping/pong mechanism directly to the target: EdmondChuiHW@8234bce

  • This commit should be sent as a patch to the upstream devtools-frontend repo. I've reviewed the contribution guide.
  • This commit is React Native-specific and cannot be upstreamed.

Copy link

@huntie huntie left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@huntie huntie merged commit 7cd81c1 into facebookexperimental:main Jun 26, 2024
3 checks passed
@EdmondChuiHW EdmondChuiHW deleted the reconnect-devtools branch June 26, 2024 09:42
@motiz88
Copy link

motiz88 commented Jun 26, 2024

@EdmondChuiHW, how is this upstreamable? The code is in the rn_fusebox.ts file which literally does not exist upstream.

Can you please follow up with an upstreamable approach and discuss it with the Chrome team?

@EdmondChuiHW
Copy link
Author

@EdmondChuiHW, how is this upstreamable? The code is in the rn_fusebox.ts file which literally does not exist upstream.

Can you please follow up with an upstreamable approach and discuss it with the Chrome team?

You're right it's not upstream-able. It's upstream-able in principle (only) but not as-is. The text is right but the checkbox is wrong; updated now. I'll need to hash out Fusebox-specific features first before proposing the right abstractions to the Chrome team.

This PR is upstream-able in principle. However, a more sophisticated mechanism to check and display connection status could be Fusebox-specific in the future, e.g. a ping/pong mechanism directly to the target: EdmondChuiHW@8234bce

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants