Skip to content

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
Member

@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
Member

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
Member

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
Member

@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 facebook: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