feat(console): Update request console sdk access modal and button #105612
Conversation
github-actions
bot
added
the
Scope: Frontend
…for-testing' into alexsohn/feat/add-request-console-sdk-access-button-component
alexsohn1126
changed the title
alexsohn1126
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
static/app/components/onboarding/gettingStartedDoc/utils/consoleExtensions.tsx
Show resolved
Hide resolved
| navigate(newPath, {replace: true}); | ||
| openPrivateGamingSdkAccessModal(modalProps); | ||
| } | ||
| }, [location.search, location.pathname, navigate, modalProps]); |
There was a problem hiding this comment.
Unstable object reference causes repeated useEffect execution
The buttonProps object in RequestSdkAccessButton is created fresh on every render and passed to useReopenGamingSdkModal, where it's included in the useEffect dependency array as modalProps. Since the object reference changes on every render, the effect runs on every render. If the component re-renders while reopenGamingSdkModal=true is still in the URL (before the navigate call updates the location), the modal could be opened multiple times. This is especially problematic in React StrictMode where effects run twice. The modalProps object should be memoized using useMemo to ensure a stable reference.
Additional Locations (1)
There was a problem hiding this comment.
I think this is legit (kinda) -- the useEffect is really reactive to location.search changes, but since you delete the search param that would trigger the open modal, this doesn't matter.
I think to make it a bit clearer, you could either memoize the buttonProps for the caller or you can do remove modalProps from the dependency array by using a ref to save its latest value e.g.
const modalPropsRef = useRef(modalprops);
modalPropsRef.current = modalProps;
useEffect(() => {
...
openPrivateGamingSdkAccessModal(modalPropsRef.current);
...
|
@sentry review |
| const metadata = CONSOLE_PLATFORM_METADATA[platform]; | ||
| return ( | ||
| <li key={platform}> | ||
| <a href={metadata?.repoURL} target="_blank" rel="noopener noreferrer"> |
There was a problem hiding this comment.
We have an ExternalLink component for this iirc.
| onClick={() => { | ||
| const separator = currentPath.includes('?') ? '&' : '?'; | ||
| const pathWithReopenFlag = `${currentPath}${separator}reopenGamingSdkModal=true`; | ||
| window.location.href = `/identity/login/github/?next=${encodeURIComponent(pathWithReopenFlag)}`; | ||
| }} | ||
| > | ||
| {t('Log in with GitHub')} | ||
| </Button> |
There was a problem hiding this comment.
What is supposed to happen here when you click it, does it open in a new window or...?
There was a problem hiding this comment.
It will trigger the GitHub App authorization flow. It is essentially the same thing as log in with GitHub, and it will link a GitHub account with Sentry account.
The GitHub page will show up in the current window. See this video:
https://github.com/user-attachments/assets/4a682e89-452e-446a-817a-8dcf9db6e781
| setSubmittedPlatforms(successfulPlatforms); | ||
| setShowSuccessView(true); |
There was a problem hiding this comment.
This may not be necessary (i'm not 100% sure), but I think you can use the result of useMutation for this instead of separate states.
e.g. if we have mutation = useMutation()... mutation.isSuccess == showSuccessView
Having the add*Message() in the handlers makes sense - but otherwise I think we can derive everything else from mutation?
| const searchParams = new URLSearchParams(location.search); | ||
| if (searchParams.get('reopenGamingSdkModal') === 'true') { | ||
| searchParams.delete('reopenGamingSdkModal'); | ||
| const newSearch = searchParams.toString(); | ||
| const newPath = location.pathname + (newSearch ? `?${newSearch}` : ''); |
There was a problem hiding this comment.
I would look into using nuqs here maybe
| navigate(newPath, {replace: true}); | ||
| openPrivateGamingSdkAccessModal(modalProps); | ||
| } | ||
| }, [location.search, location.pathname, navigate, modalProps]); |
There was a problem hiding this comment.
I think this is legit (kinda) -- the useEffect is really reactive to location.search changes, but since you delete the search param that would trigger the open modal, this doesn't matter.
I think to make it a bit clearer, you could either memoize the buttonProps for the caller or you can do remove modalProps from the dependency array by using a ref to save its latest value e.g.
const modalPropsRef = useRef(modalprops);
modalPropsRef.current = modalProps;
useEffect(() => {
...
openPrivateGamingSdkAccessModal(modalPropsRef.current);
...
Previously the console sdk access modal looked like this:
This would create a user feedback that will land in a slack channel, and that would need to be manually put into SaC repo.
This PR changes the modal to the following:
When user has no GitHub identity linked to their Sentry account:
When user has a GitHub identity linked to their Sentry account:
After inviting themselves into a repository:
See this video for what it would look like for the user:
https://github.com/user-attachments/assets/4a682e89-452e-446a-817a-8dcf9db6e781