feat(codecov): Add onboarding radio selector for GHA or CLI #90980
Conversation
github-actions
bot
added
the
Scope: Frontend
calvin-codecov
force-pushed
the
cy/codecov_onboarding_radio
branch
from
36eddd0 to
03da34c
Compare
calvin-codecov
force-pushed
the
cy/codecov_onboarding_radio
branch
from
03da34c to
fa12f3f
Compare
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
| )} | ||
| </TAValueText> | ||
| </IntroContainer> | ||
| <SelectOptionHeader>Select a setup option</SelectOptionHeader> |
|
|
||
| export default function TestsOnboardingPage() { | ||
| const [searchParams, setSearchParams] = useSearchParams(); |
There was a problem hiding this comment.
I'm curious as to why we're using the url to handle state vs a local useState. Are we expecting to reach the /new route with a url param that could be any of these two? And what is the value of exposing the opt to the customer via the URL vs having it in an internal state?
There was a problem hiding this comment.
Hmm...I swear I read somewhere that a requirement was to have the url change so that linking to it would preset the option. Now I'm having a hard time finding where I saw that though... Gonna have to do a little bit of digging
There was a problem hiding this comment.
Ahh, it was on this ticket https://linear.app/getsentry/issue/CCMRG-150/test-analytics-onboarding-create-radio-selection. Seems like we may have two similar/duplicate tickets for the radio selector that were created at different times. This is the other one I have included in the PR description https://linear.app/getsentry/issue/CCMRG-426/add-radio-selector-for-choosing-between-gh-actions-and-cli
There was a problem hiding this comment.
Ahh gotcha gotcha, ok sounds good! Yeap if this is expected to be part of the URL then this approach is good.
Should the url be populated with the default value if one isn't provided?
There was a problem hiding this comment.
Do you see any benefits we would get with the default value being added? I was thinking of setting the searchParams on mount to add the query param by default but that forces a remount which I think may not be worth it if it already works when no default is specified.
There was a problem hiding this comment.
Mostly it being in sync with the app's state. When you click on another entry it syncs, which is nice, but I suppose if you default to an option it's okay as long as there's a default for blank.
| initialRouterConfig: { | ||
| location: { | ||
| pathname: '/codecov/tests/new', | ||
| query: {opt: ''}, |
There was a problem hiding this comment.
I think it'd be worth adding a test with query being empty/null; subtle but different is that the radio button will have the default value (gha) but the opt will be '' (or undefined if it's just /new), leaving the url and button out of sync. Sort of tied to my other comment of having the url be the state vs a useState hook
calvin-codecov
force-pushed
the
cy/codecov_onboarding_radio
branch
from
04b47dd to
e8695d6
Compare
Closes https://linear.app/getsentry/issue/CCMRG-426/add-radio-selector-for-choosing-between-gh-actions-and-cli 
Closes https://linear.app/getsentry/issue/CCMRG-426/add-radio-selector-for-choosing-between-gh-actions-and-cli