feat(scm-first-messaging-integration): Rendering provider rows - #121768
feat(scm-first-messaging-integration): Rendering provider rows#121768Abdkhan14 wants to merge 12 commits into
Conversation
github-actions
Bot
added
the
Scope: Frontend
Abdkhan14
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| </Flex> | ||
|
|
||
| {isPending && ( | ||
| <Stack gap="md"> |
There was a problem hiding this comment.
medium: These placeholders don't match the height of the real rows, produces a bit of a jarring change when loading is complete.
There was a problem hiding this comment.
Resorted to just a spinner like the other pages for now
| <Stack gap="xl" maxWidth={`min(${SCM_STEP_CONTENT_WIDTH}, 100%)`} width="100%"> | ||
| <Stack gap="md"> | ||
| <Heading as="h2" size="4xl"> | ||
| <Stack gap="2xl" maxWidth={`min(${SCM_STEP_CONTENT_WIDTH}, 100%)`} width="100%"> |
There was a problem hiding this comment.
medium: Not sure that the text sizes / spacings are consistent with the other steps, if it were me I think I would copy the Header / subheader and info copy sizes and spacings used on the "Connect your code" step, or ask Dan G if things are intentionally divergent from the other steps.
There was a problem hiding this comment.
Synced with Dan, agreed to iterate on this once we have it live behind the flag
|
|
||
| {!isPending && !isError && providers.length > 0 && ( | ||
| <Stack gap="md"> | ||
| {providers.map(viewModel => ( |
There was a problem hiding this comment.
medium: I'm a little bit confused by how this operates relative to the designs, i.e. in the design:
- when no providers are connected all provider rows are shown
- when slack is connected, but alerts aren't configured for it only slack is shown
- when slack is connected and alerts are configured for it, all provider rows are shown
In your version though, all provider rows are always shown.
might be worth confirming with dan / aligning this to the designs.
There was a problem hiding this comment.
Started a thread for this with Dan, agreed to iterate on this once we have it live behind the flag
| <Alert | ||
| variant="warning" | ||
| trailingItems={ | ||
| <Alert.Button onClick={refetchIntegrations}>{t('Retry')}</Alert.Button> |
There was a problem hiding this comment.
medium: isError includes provider-query failures, but Retry only refetches the integrations query. a failed /config/integrations/ request therefore leaves this alert stuck. could the hook expose a retry that refetches both query sets?
There was a problem hiding this comment.
Pushed a change for this. The hook now exposes a retry() that refetches both query sets (the integrations query and all three /config/integrations/ provider queries via a refetch surfaced from the useQueries combine), and the load-error Alert calls that instead of refetchIntegrations. So a config-query failure can now recover. Kept refetchIntegrations for the post-install path since installing doesn't change provider config. Added a spec where a provider config query 500s and retry() clears the error
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a28c4ba. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| @@ -90,7 +105,7 @@ function deriveVisualState({ | |||
| } | |||
There was a problem hiding this comment.
medium: awaitingInstall only gets cleared if the integration surfaces. if the post-install refetch succeeds but still returns no integration, installState stays complete and this row will keep loading with no way to retry. could the parent pass isRefetching so we show loading only while the refetch is active, then fall back to Connect once it settles?
There was a problem hiding this comment.
Fixed — the row now uses the parent’s isRefetchingIntegrations state instead of the awaitingInstall latch. It shows loading only during the refetch and returns to Connect if no integration surfaces. Added regression coverage for this case
2 participants
Problem
The earlier PRs in this stack built the pieces of the inline messaging step in isolation — the provider data hook, the destination-validation hook, the provider row, and the channel picker — but nothing actually assembled them into the
ScmMessagingonboarding step. The step didn't render the provider rows, had no loading/error handling while integrations were fetching, and no submit gating tied to whether a chosen destination was actually valid. The row and picker also still carried interim UX (a manual "Add destination" click to reveal the picker, a reachable but redundantconnectedstate) and placeholder copy/layout that didn't match the approved design.Change
Composes the full step in
scmMessaging.tsx: it renders oneScmMessagingProviderRowper curated provider with loading skeletons and an error/retry state, shows the validation banner for a restored destination, and wires the footer (Back, Set up later, Continue). Continue is gated oncanContinue— a destination must be saved and revalidation must have settled clean (no stale reason, not erroring, not pending) — so a stale or unverified destination can never look submittable, which matters because Continue is the project/alert-rule creation boundary. The row now auto-expands the channel picker as soon as an integration connects (removing the extra click and the now-unreachableconnectedstate), and only offers Cancel in edit mode. The picker moves to a two-column Workspace/Channel layout,