Skip to content

Conversation

@EdmondChuiHW
Copy link

@EdmondChuiHW EdmondChuiHW commented Mar 26, 2024

Summary

Minor tweaks to help dogfooders read docs and send feedback.

  • add feedbackLink to experiment
  • allow dynamic feedbackLink from globalThis for alternative URLs such as FB internal feedback form
  • show feedbackLink from RN Welcome panel

Stack

Test plan

With globalThis.reactNativeFeedbackLink = 'https://example.com':

image

image

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.

@EdmondChuiHW EdmondChuiHW changed the title Feedbacklink Add feedback link support Mar 26, 2024
@EdmondChuiHW EdmondChuiHW changed the title Add feedback link support Add feedbackLink to experiment Mar 26, 2024
@EdmondChuiHW EdmondChuiHW marked this pull request as ready for review March 26, 2024 03:10
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.

Unlike the previous PR, I don't think we should implement this link — motivation being that we won't provide this for OSS (unless perhaps we'd want to give Expo the hook to do so).

Let's double-down on the Bugnub in your next PR as the only (optionally set) CTA for feedback.

@EdmondChuiHW
Copy link
Author

Ah I should have been more clear in the summary about this feedback link being optional.

In other words, landing this PR is no-op for open-source. The feedback link has no default if the global var isn't set. And thus the link in the welcome screen would not be shown if it's not present.

Bottom line: without the globalThis.reactNativeFeedbackLink = 'https://example.com' equivalent from our internal embedder script, this PR is no-op.

I initially thought about using custom labels, e.g. [FB-only] Send feedback, if we have an appetite for another global var (and making it work with i18n in CDT).

Or we can also prepend an unstable_ prefix to the global vars if we want to signal this to OSS (and hardcode the label to read [FB-only] Send feedback)

Happy to adjust if there's a preference to hide this all together in the experiment or RN Welcome page. While the toolbar button in the next PR is present across all panels, showing doc and feedback links together seems like a CDT UI pattern (and makes sense to me too)

@EdmondChuiHW EdmondChuiHW requested a review from huntie March 26, 2024 18:28
@EdmondChuiHW
Copy link
Author

Discussed offline. No diff for settings or RNWelcome. Closing PR

@EdmondChuiHW EdmondChuiHW deleted the feedbacklink branch May 29, 2024 13:10
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