ref(project-creation): Extract shared hooks from the notification components#118581
Merged
Conversation
github-actions
Bot
added
the
Scope: Frontend
Contributor
📊 Type Coverage Diff
🔍 5 new type safety issues introduced
Non-null assertions (
This is informational only and does not block the PR. |
jaydgoss
changed the title
jaydgoss
force-pushed
the
jaygoss/scm-alert-notification-options
branch
from
7f153aa to
37b2315
Compare
jaydgoss
force-pushed
the
jaygoss/extract-messaging-alert-rule-hook
branch
from
336815c to
5a20cb4
Compare
Contributor
Sentry Snapshot Testing
|
jaydgoss
force-pushed
the
jaygoss/scm-alert-notification-options
branch
from
37b2315 to
be81f9e
Compare
jaydgoss
force-pushed
the
jaygoss/extract-messaging-alert-rule-hook
branch
from
5a20cb4 to
bd1be2b
Compare
Comment on lines
+103
to
+135
| return { | ||
| provider, | ||
| integration, | ||
| channel, | ||
| providerOptions, | ||
| integrationOptions, | ||
| channelOptions, | ||
| isChannelLoading: isPending || validateChannel.isFetching, | ||
| channelError: validateChannel.error, | ||
| providerDisabled: Object.keys(providersToIntegrations).length === 1, | ||
| integrationDisabled: integrationOptions.length === 1, | ||
| onProviderChange: (option: any) => { | ||
| setProvider(option.value); | ||
| setIntegration(providersToIntegrations[option.value]![0]); | ||
| setChannel(undefined); | ||
| validateChannel.clear(); | ||
| }, | ||
| onIntegrationChange: (option: any) => { | ||
| setIntegration(option.value); | ||
| setChannel(undefined); | ||
| validateChannel.clear(); | ||
| }, | ||
| onChannelChange: (option: {label: React.ReactNode; value: string} | null) => { | ||
| setChannel( | ||
| option ? {value: option.value, label: option.label, new: false} : undefined | ||
| ); | ||
| validateChannel.clear(); | ||
| }, | ||
| onCreateChannel: (newOption: string) => { | ||
| setChannel({value: newOption, label: newOption, new: true}); | ||
| }, | ||
| }; | ||
| } |
Member
There was a problem hiding this comment.
wondering if we should have an explicit type with comments for this stuff. Just a though, not a requirement
evanpurkhiser
approved these changes
Jun 26, 2026
jaydgoss
force-pushed
the
jaygoss/scm-alert-notification-options
branch
from
be81f9e to
fb1f238
Compare
jaydgoss
force-pushed
the
jaygoss/extract-messaging-alert-rule-hook
branch
from
bd1be2b to
37e423c
Compare
jaydgoss
force-pushed
the
jaygoss/scm-alert-notification-options
branch
from
fb1f238 to
050e09e
Compare
jaydgoss
force-pushed
the
jaygoss/extract-messaging-alert-rule-hook
branch
from
37e423c to
66f5541
Compare
jaydgoss
force-pushed
the
jaygoss/scm-alert-notification-options
branch
from
050e09e to
0131e54
Compare
jaydgoss
force-pushed
the
jaygoss/extract-messaging-alert-rule-hook
branch
from
66f5541 to
710919e
Compare
jaydgoss
force-pushed
the
jaygoss/scm-alert-notification-options
branch
from
0131e54 to
7d5e693
Compare
jaydgoss
force-pushed
the
jaygoss/extract-messaging-alert-rule-hook
branch
from
710919e to
f00a0c4
Compare
Base automatically changed from
jaygoss/scm-alert-notification-options
to
master
June 30, 2026 16:40
Pull the channel query, channel validation, and the provider/integration/channel options and change handlers out of MessagingIntegrationAlertRule into a shared useMessagingIntegrationAlertRule hook, and export ChannelSelect and ChannelField. The classic component consumes the hook and renders the same inline card. This lets an SCM-styled rule reuse the logic without duplicating it.
The hook, ChannelSelect, and ChannelField are exported for reuse by the SCM layout, which lands in a stacked PR. Tag them @public so knip does not flag them as unused while their only external consumer is still downstream.
Pull the shared shell out of IssueAlertNotificationOptions into a useIssueAlertNotificationOptions hook: the shouldRenderNotificationConfigs derivation, the setup-button impression analytics, and the querySuccess gate. The SCM layout can reuse the same logic and analytics instead of duplicating them. The classic component consumes the hook and renders identically.
jaydgoss
force-pushed
the
jaygoss/extract-messaging-alert-rule-hook
branch
from
f00a0c4 to
ea55b39
Compare
jaydgoss
added a commit
that referenced
this pull request
Jun 30, 2026
## TLDR Add an SCM-styled version of the notification options for the alert-frequency section. The messaging-integration rule lays out one sentence fragment per row with full-width inputs and no grey card background, and reveals with the shared collapse animation. Builds on #118581. ## Details - Add `ScmMessagingIntegrationAlertRule`, which reuses the `useMessagingIntegrationAlertRule` hook (from #118581) and renders `makeSentence` in a flat `Flex` column, so each fragment ("Send", the provider select, "notification to the", and so on) sits on its own full-width row, without the `background.secondary` card styling. - Add `ScmIssueAlertNotificationOptions`: a "Notify via" header above "Email" and "Integration (Slack, Discord, MS Teams, etc.)" checkboxes that renders the stacked rule, replacing the direct use of the shared picker in `ScmAlertFrequencySection`. The checkboxes are composed directly from the scraps `Checkbox` (not `MultipleCheckbox`) so they stack full width. - The rule reveals with `ScmCollapsibleReveal` (height and fade) when Integration is checked and indents under its checkbox label. The long Integration label and the section footer truncate with an ellipsis on narrow screens.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Pull the reusable logic out of the classic notification components into shared hooks so an SCM-styled layout can reuse it. No behavior change to the classic project-creation flow.
Details
useMessagingIntegrationAlertRule(the channel query, validation, and the provider/integration/channel options and handlers) fromMessagingIntegrationAlertRule, and exportChannelSelectandChannelField. The classic rule consumes the hook and renders the same inline grey card.useIssueAlertNotificationOptionsfromIssueAlertNotificationOptions(theshouldRenderNotificationConfigsderivation, the setup-button impression analytics, and thequerySuccessgate). The classic component consumes the hook and renders identically.@publicso knip doesn't flag them while their second consumer (the SCM layout) is still in the PR stacked on top of this one.