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 new 'open externally' button #134

Merged

Conversation

EdmondChuiHW
Copy link

@EdmondChuiHW EdmondChuiHW commented Nov 12, 2024

Summary

Add a new button to open the source file at the current line in an external editor , e.g. VS Code

image

This is current not supported in the community version of Metro or Fusebox.

Test plan

Requires D65551943

  1. Button shows up on a source file upon hovering the tab
  2. Clicking the button will trigger Metro to open the file
    image
  3. The file should be opened in the editor with the same line number selected from CDT

Upstreaming plan

  • 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.

Summarising offline feedback: I think we can improve the UX/discoverability of this, and minimise code inserts, by relocating this button.

image

@EdmondChuiHW
Copy link
Author

EdmondChuiHW commented Nov 12, 2024

Updated design based on @huntie's feedback 🙌

Default:
screenshot of the new button with tooltip

Custom:
screenshot of the new button with VS Code icon and tooltip

Comment on lines 12 to 14
var enableReactNativeOpenSourceFilesInExternalEditor: boolean|undefined;
var reactNativeOpenSourceFilesInExternalEditorButtonBackgroundImage: string|undefined;
var reactNativeOpenSourceFilesInExternalEditorButtonPadding: string|undefined;
Copy link

Choose a reason for hiding this comment

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

Paired with my other feedback on D65551943.

Suggested change
var enableReactNativeOpenSourceFilesInExternalEditor: boolean|undefined;
var reactNativeOpenSourceFilesInExternalEditorButtonBackgroundImage: string|undefined;
var reactNativeOpenSourceFilesInExternalEditorButtonPadding: string|undefined;
var enableReactNativeOpenInExternalEditor: boolean|undefined; // Also, deletable if we align /open-stack-frame
var reactNativeOpenInEditorButtonImage: string|undefined;

adorner.style.setProperty('background-image', maybeBackgroundImage);
adorner.style.setProperty(
'padding',
globalThis.reactNativeOpenSourceFilesInExternalEditorButtonPadding ?? '4px',
Copy link

Choose a reason for hiding this comment

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

Please kill, see D65551943 comment.

@EdmondChuiHW
Copy link
Author

EdmondChuiHW commented Nov 13, 2024

Revised:

Default:

screenshot of the button with default glyph

Custom:

screenshot of the button with custom icon

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.

🚀

@EdmondChuiHW EdmondChuiHW merged commit b61aae3 into facebookexperimental:main Nov 13, 2024
2 checks passed
@EdmondChuiHW EdmondChuiHW deleted the open-externally branch November 13, 2024 18:20
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.

3 participants