feat: introducing new feature 'Notifications' #1962
Conversation
…, update constants
WalkthroughAdds a notifications subsystem: route entries and start path, popup and fullscreen UIs, popup/fullscreen settings, notification types/constants, Subscan fetch helpers, utilities for formatting/grouping, a reducer-driven Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Header / Settings UI
participant Router as Router / Popups
participant Page as Notification Page/Popup
participant Hook as useNotifications
participant Storage as Storage
User->>UI: Click Notifications
UI->>Router: navigate("/notification/") or open popup
Router->>Page: render Notification UI
Page->>Hook: initialize/useNotifications()
Hook->>Storage: load settings & saved notifications
Hook-->>Page: notifications, grouped items, status flags
Page->>Hook: markAsRead() / openSettings()
Hook->>Storage: persist updates
sequenceDiagram
autonumber
participant Hook as useNotifications
participant Fetch as SubscanHelpers
participant Utils as util.generate*/group
participant Storage as Storage
Hook->>Storage: load saved state
alt notifications enabled
Hook->>Fetch: getReceivedFundsInformation(...)
Hook->>Fetch: getPayoutsInformation(...)
Hook->>Fetch: getReferendasInformation(...)
Fetch-->>Hook: data
Hook->>Utils: generate*Notifications(data)
Utils-->>Hook: messages
Hook->>Utils: groupNotificationsByDay(messages)
Utils-->>Hook: groupedNotifications
Hook->>Storage: enqueue save(state)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
packages/extension-polkagate/src/components/TwoToneText.tsx (1)
16-16: Consider removing unnecessary object spread.Both lines use
{ ...style }, but React'sstyleattribute accepts the object directly. The spread creates a new object on each render with no benefit here.Apply this diff to simplify:
if (!textPartInColor) { - return <span style={{ ...style }}>{text}</span>; + return <span style={style}>{text}</span>; } return ( <span dangerouslySetInnerHTML={{ __html: text.replace( textPartInColor, `<span style="color: ${color}; ${ backgroundColor ? `background-color: ${backgroundColor}; border-radius: 6px; padding: 0 5px 0` : '' }">${textPartInColor}</span>` ) }} - style={{ ...style }} + style={style} /> );Also applies to: 29-29
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
packages/extension-base/src/defaults.ts(1 hunks)packages/extension-polkagate/src/components/ActionCard.tsx(4 hunks)packages/extension-polkagate/src/components/TwoToneText.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/components/layout/Notifications.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/notification/index.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/notification/partials/SelectAccount.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/settings/AccountSettings.tsx(4 hunks)packages/extension-polkagate/src/fullscreen/settings/NetworkSettings.tsx(1 hunks)packages/extension-polkagate/src/hooks/index.ts(1 hunks)packages/extension-polkagate/src/hooks/useChainInfo.ts(1 hunks)packages/extension-polkagate/src/hooks/useNotifications.ts(1 hunks)packages/extension-polkagate/src/hooks/useTokenPriceBySymbol.ts(1 hunks)packages/extension-polkagate/src/partials/UserDashboardHeader.tsx(2 hunks)packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/constant.ts(1 hunks)packages/extension-polkagate/src/popup/notification/helpers.ts(1 hunks)packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts(1 hunks)packages/extension-polkagate/src/popup/notification/index.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/partials/AccountToggle.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/partials/ChainToggle.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/partials/NotificationGroup.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/partials/Partial.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/partials/SelectAccount.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/partials/SelectChain.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/types.ts(1 hunks)packages/extension-polkagate/src/popup/notification/util.ts(1 hunks)packages/extension-polkagate/src/popup/settings/accountSettings/index.tsx(2 hunks)packages/extension-polkagate/src/popup/settings/index.tsx(1 hunks)packages/extension-polkagate/src/util/constants.ts(2 hunks)packages/extension-ui/src/Popup/contexts/AccountAssetProvider.tsx(2 hunks)packages/extension-ui/src/Popup/routes/featuresRoutes.ts(2 hunks)packages/extension/package.json(1 hunks)packages/extension/public/locales/en/translation.json(1 hunks)packages/extension/src/packageInfo.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (22)
packages/extension-polkagate/src/fullscreen/notification/partials/SelectAccount.tsx (2)
packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/popup/notification/constant.ts (1)
MAX_ACCOUNT_COUNT_NOTIFICATION(19-19)
packages/extension-polkagate/src/hooks/useNotifications.ts (10)
packages/extension-polkagate/src/popup/notification/types.ts (2)
NotificationsType(146-153)NotificationActionType(155-161)packages/extension-polkagate/src/popup/notification/util.ts (6)
markMessagesAsRead(220-227)generateReferendaNotifications(86-141)updateReferendas(235-290)generateReceivedFundNotifications(184-213)generateStakingRewardNotifications(148-177)groupNotificationsByDay(304-364)packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (1)
useNotificationSettings(65-176)packages/extension-polkagate/src/hooks/useSelectedChains.ts (1)
useSelectedChains(14-31)packages/extension-polkagate/src/hooks/useGenesisHashOptions.ts (1)
useGenesisHashOptions(14-63)packages/extension-polkagate/src/util/types.ts (1)
DropdownOption(877-880)packages/extension-polkagate/src/util/constants.ts (1)
STORAGE_KEY(310-325)packages/extension-polkagate/src/popup/notification/constant.ts (2)
AUTO_MARK_AS_READ_DELAY(29-29)SUBSCAN_SUPPORTED_CHAINS(50-78)packages/extension-polkagate/src/util/chain.ts (1)
sanitizeChainName(7-26)packages/extension-polkagate/src/popup/notification/helpers.ts (3)
getReceivedFundsInformation(111-179)getPayoutsInformation(187-266)getReferendasInformation(273-336)
packages/extension-polkagate/src/fullscreen/components/layout/Notifications.tsx (3)
packages/extension-polkagate/src/hooks/useIsExtensionPopup.ts (1)
useIsExtensionPopup(4-21)packages/extension-polkagate/src/util/handleExtensionPopup.ts (1)
useExtensionPopups(11-18)packages/extension-polkagate/src/hooks/useNotifications.ts (1)
useNotifications(107-301)
packages/extension-polkagate/src/popup/notification/index.tsx (2)
packages/extension-polkagate/src/hooks/useNotifications.ts (1)
useNotifications(107-301)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx (5)
packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (2)
NotificationSettingType(12-18)useNotificationSettings(65-176)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx (1)
SelectChain(20-71)packages/extension-polkagate/src/popup/notification/constant.ts (2)
SUPPORTED_GOVERNANCE_NOTIFICATION_CHAIN(47-47)SUPPORTED_STAKING_NOTIFICATION_CHAIN(48-48)packages/extension-polkagate/src/fullscreen/components/DraggableModal.tsx (1)
DraggableModal(30-172)
packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx (3)
packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx (1)
TextValuePair(21-24)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/util/chain.ts (1)
sanitizeChainName(7-26)
packages/extension/src/packageInfo.ts (5)
packages/extension-base/src/packageInfo.ts (1)
packageInfo(6-6)packages/extension-chains/src/packageInfo.ts (1)
packageInfo(6-6)packages/extension-compat-metamask/src/packageInfo.ts (1)
packageInfo(6-6)packages/extension-dapp/src/packageInfo.ts (1)
packageInfo(6-6)packages/extension-inject/src/packageInfo.ts (1)
packageInfo(6-6)
packages/extension-polkagate/src/fullscreen/notification/index.tsx (3)
packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/hooks/useNotifications.ts (1)
useNotifications(107-301)packages/extension-polkagate/src/fullscreen/components/DraggableModal.tsx (1)
DraggableModal(30-172)
packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (3)
packages/extension-polkagate/src/components/contexts.tsx (1)
AccountContext(34-34)packages/extension-polkagate/src/util/constants.ts (1)
STORAGE_KEY(310-325)packages/extension-polkagate/src/popup/notification/constant.ts (3)
DEFAULT_NOTIFICATION_SETTING(31-37)MAX_ACCOUNT_COUNT_NOTIFICATION(19-19)SET_UP_NOTIFICATION_SETTING(39-45)
packages/extension-polkagate/src/popup/notification/constant.ts (1)
packages/extension-polkagate/src/util/constants.ts (4)
POLKADOT_GENESIS_HASH(58-58)KUSAMA_GENESIS_HASH(59-59)WESTEND_GENESIS_HASH(60-60)PASEO_GENESIS_HASH(61-61)
packages/extension-ui/src/Popup/routes/featuresRoutes.ts (1)
packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx (1)
NotificationSettings(28-145)
packages/extension-polkagate/src/popup/notification/helpers.ts (9)
packages/extension-polkagate/src/popup/notification/types.ts (10)
TransferSubscan(14-38)TransfersProp(78-87)PayoutSubscan(40-50)PayoutsProp(89-94)ReferendaSubscan(52-62)ReferendaProp(96-107)ReceivedFundInformation(109-113)ApiResponse(7-12)StakingRewardInformation(115-119)ReferendaInformation(121-124)packages/extension-polkagate/src/util/types.ts (1)
DropdownOption(877-880)packages/extension-polkagate/src/util/address.ts (1)
getSubstrateAddress(40-59)packages/extension-polkagate/src/popup/notification/util.ts (1)
timestampToDate(16-51)packages/extension-polkagate/src/popup/notification/constant.ts (5)
ReferendaStatus(6-6)BATCH_SIZE(17-17)MAX_RETRIES(16-16)REFERENDA_COUNT_TO_TRACK_DOT(10-10)REFERENDA_COUNT_TO_TRACK_KSM(11-11)packages/extension-polkagate/src/util/getChainName.ts (1)
getChainName(15-29)packages/extension-polkagate/src/util/chain.ts (1)
getSubscanChainName(32-54)packages/extension-polkagate/src/util/api/postData.ts (1)
postData(6-25)packages/extension-polkagate/src/util/constants.ts (2)
POLKADOT_GENESIS_HASH(58-58)KUSAMA_GENESIS_HASH(59-59)
packages/extension-polkagate/src/popup/notification/partials/Partial.tsx (3)
packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/components/ActionButton.tsx (1)
ActionButton(30-124)packages/extension-polkagate/src/util/constants.ts (1)
EXTENSION_NAME(6-6)
packages/extension-polkagate/src/popup/notification/types.ts (2)
packages/extension-polkagate/src/popup/notification/constant.ts (1)
ReferendaStatus(6-6)packages/extension-polkagate/src/util/types.ts (1)
DropdownOption(877-880)
packages/extension-polkagate/src/popup/notification/partials/SelectChain.tsx (4)
packages/extension-polkagate/src/util/handleExtensionPopup.ts (1)
ExtensionPopupCloser(9-9)packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx (1)
TextValuePair(21-24)packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx (1)
SelectChain(20-71)packages/extension-polkagate/src/util/chain.ts (1)
sanitizeChainName(7-26)
packages/extension-polkagate/src/popup/notification/util.ts (5)
packages/extension-polkagate/src/popup/notification/types.ts (6)
ReferendaInformation(121-124)NotificationMessageType(135-144)StakingRewardInformation(115-119)ReceivedFundInformation(109-113)NotificationType(133-133)ReferendaProp(96-107)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/hooks/useChainInfo.ts (1)
ChainInfo(25-32)packages/extension-polkagate/src/hooks/useTokenPriceBySymbol.ts (1)
Price(17-20)packages/extension-polkagate/src/fullscreen/home/partials/type.ts (1)
CurrencyItemType(4-9)
packages/extension-polkagate/src/popup/notification/partials/NotificationGroup.tsx (8)
packages/extension-polkagate/src/popup/notification/util.ts (5)
isToday(373-382)getNotificationItemTitle(473-502)getTimeOfDay(390-400)getNotificationDescription(504-559)getNotificationIcon(561-585)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/hooks/useAccount.ts (1)
useAccount(12-28)packages/extension-polkagate/src/util/address.ts (1)
toShortAddress(61-69)packages/extension-polkagate/src/popup/notification/types.ts (1)
NotificationMessageType(135-144)packages/extension-polkagate/src/hooks/useCurrency.ts (1)
useCurrency(17-33)packages/extension-polkagate/src/hooks/useChainInfo.ts (1)
useChainInfo(42-73)packages/extension-polkagate/src/hooks/useTokenPriceBySymbol.ts (1)
useTokenPriceBySymbol(31-54)
packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx (4)
packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (1)
useNotificationSettings(65-176)packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx (1)
SelectChain(20-71)packages/extension-polkagate/src/popup/notification/constant.ts (2)
SUPPORTED_GOVERNANCE_NOTIFICATION_CHAIN(47-47)SUPPORTED_STAKING_NOTIFICATION_CHAIN(48-48)
packages/extension-ui/src/Popup/contexts/AccountAssetProvider.tsx (1)
packages/extension-polkagate/src/hooks/useNotifications.ts (1)
useNotifications(107-301)
packages/extension-polkagate/src/popup/notification/partials/SelectAccount.tsx (3)
packages/extension-polkagate/src/util/handleExtensionPopup.ts (1)
ExtensionPopupCloser(9-9)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/popup/notification/constant.ts (1)
MAX_ACCOUNT_COUNT_NOTIFICATION(19-19)
packages/extension-polkagate/src/popup/notification/partials/AccountToggle.tsx (1)
packages/extension-polkagate/src/util/constants.ts (1)
POLKADOT_GENESIS_HASH(58-58)
packages/extension-polkagate/src/hooks/useChainInfo.ts (1)
packages/extension-polkagate/src/util/types.ts (1)
ChainInfo(317-324)
🔇 Additional comments (5)
packages/extension-polkagate/src/hooks/useTokenPriceBySymbol.ts (1)
17-20: LGTM!Exporting the
Priceinterface is correct and enables the notification modules to import and reuse this type consistently across the codebase.packages/extension-polkagate/src/partials/UserDashboardHeader.tsx (2)
45-48: LGTM! Clean horizontal layout implementation.The Stack wrapper effectively arranges the Notifications and FullscreenModeButton controls side-by-side with appropriate alignment and spacing. The implementation follows Material-UI best practices.
Please verify that:
- The layout works correctly in both popup and fullscreen modes (considering the different available widths)
- Keyboard navigation flows naturally between the two controls
- The 4px gap provides sufficient visual separation between the notification icon and fullscreen button
10-10: No action needed:Notificationsis correctly imported and requires no props, handling both popup and fullscreen contexts internally.packages/extension-polkagate/src/components/TwoToneText.tsx (2)
16-16: Good fix—now consistently respects thestyleprop.Previously, passing
stylewithouttextPartInColorhad no effect. This change ensures the prop is always honored.
19-31: Verify safety oftextandtextPartInColorinputs
Ensure every value passed intoTwoToneTextis sanitized or comes from a trusted source to prevent XSS viadangerouslySetInnerHTML.
packages/extension-polkagate/src/fullscreen/components/layout/Notifications.tsx
Show resolved
Hide resolved
packages/extension-polkagate/src/popup/notification/partials/NotificationGroup.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
packages/extension-polkagate/src/popup/notification/constant.ts (1)
6-13: FixReferendaStatuscasing to align with Subscan data
The union still exposes'timeout', but every consumer (and Subscan’s payload) uses'timedOut'. Because of this mismatch we end up storing'timedOut'while the type only allows'timeout', so the value never lines up with the constant list or downstream comparisons. Please switch the literal to'timedOut'to keep the type, constants, and fetched data consistent.-export type ReferendaStatus = 'ongoing' | 'approved' | 'timeout' | 'rejected' | 'cancelled'; +export type ReferendaStatus = 'ongoing' | 'approved' | 'timedOut' | 'rejected' | 'cancelled';packages/extension-polkagate/src/hooks/useNotifications.ts (1)
140-230: Persist reducer output instead of stale snapshotsLine 148 persists the pre-dispatch
notificationsobject becausesaveNotifications()closes over the old state and you invoke it immediately after eachdispatchNotificationson Lines 195, 212, and 229. React hasn’t flushed yet, so storage misses the freshly fetched messages/read flags, and theMARK_AS_READreducer result is never saved either. Persist after the reducer settles (or pass the computed next state into the queue) and drop the direct calls so storage always reflects the reducer output.- const saveNotifications = useCallback(() => { + const enqueueSave = useCallback((next: NotificationsType) => { // Queue saves to ensure they happen sequentially, not in parallel saveQueue.current = saveQueue.current.then(async () => { if (isSavingRef.current) { return; } isSavingRef.current = true; - const dataToSave = { ...notifications, latestLoggedIn: Math.floor(Date.now() / 1000) }; + const dataToSave = { ...next, latestLoggedIn: Math.floor(Date.now() / 1000) }; try { await setStorage(STORAGE_KEY.NOTIFICATIONS, dataToSave); // console.log('✅ Notifications saved after fetch completion.'); } catch (error) { console.error('❌ Failed to save notifications:', error); } finally { isSavingRef.current = false; } }); - }, [notifications]); + }, []); + + useEffect(() => { + if (!initializedRef.current) { + return; + } + + enqueueSave(notifications); + }, [enqueueSave, notifications]);- dispatchNotifications({ - payload: receivedFunds, - type: 'SET_RECEIVED_FUNDS' - }); - - saveNotifications(); + dispatchNotifications({ + payload: receivedFunds, + type: 'SET_RECEIVED_FUNDS' + });- dispatchNotifications({ - payload: payouts, - type: 'SET_STAKING_REWARDS' - }); - - saveNotifications(); + dispatchNotifications({ + payload: payouts, + type: 'SET_STAKING_REWARDS' + });- dispatchNotifications({ - payload: referendas, - type: 'SET_REFERENDA' - }); - - saveNotifications(); + dispatchNotifications({ + payload: referendas, + type: 'SET_REFERENDA' + });packages/extension-polkagate/src/popup/notification/util.ts (2)
149-214: Emit a notification for every new payout/transferLine 155 relies on
Array.find, so each account produces at most one staking reward ≥latestLoggedIn - offset; the rest are silently ignored. The same pattern on Line 191 drops received-fund transfers. Users miss notifications whenever more than one event arrives between polls. Iterate all qualifying entries instead.- const newPayouts = payouts.map(({ address, data, network }) => { - const payout = data.find(({ timestamp }) => timestamp >= latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET); - - return payout - ? { - address, - network, - payout - } - : undefined; - }).filter((item) => !!item); - - // Generate notifications for new payouts - newPayouts.forEach(({ address, network, payout }) => { - newMessages.push({ - chain: network, - forAccount: address, - payout, - read: false, - type: 'stakingReward' - }); - }); + payouts.forEach(({ address, data, network }) => { + data + .filter(({ timestamp }) => timestamp >= latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET) + .forEach((payout) => { + newMessages.push({ + chain: network, + forAccount: address, + payout, + read: false, + type: 'stakingReward' + }); + }); + });- const newReceivedFunds = transfers.map(({ address, data, network }) => { - const receivedFund = data.find(({ timestamp }) => timestamp >= latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET); - - return receivedFund - ? { - address, - network, - receivedFund - } - : undefined; - }).filter((item) => !!item); - - // Generate notifications for new payouts - newReceivedFunds.forEach(({ address, network, receivedFund }) => { - newMessages.push({ - chain: network, - forAccount: address, - read: false, - receivedFund, - type: 'receivedFund' - }); - }); + transfers.forEach(({ address, data, network }) => { + data + .filter(({ timestamp }) => timestamp >= latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET) + .forEach((receivedFund) => { + newMessages.push({ + chain: network, + forAccount: address, + read: false, + receivedFund, + type: 'receivedFund' + }); + }); + });
14-503: Stop calling hooks from plain utilitiesLine 475 calls
useTranslation()insidegetNotificationItemTitle, which is neither a component nor a custom hook. React throws “Invalid hook call” the first time this helper runs. Thread the translator from the caller and drop the hook here; remember to update call sites to pass theirt.-import { useTranslation } from '@polkadot/extension-polkagate/src/hooks'; ... -export function getNotificationItemTitle (type: NotificationType, referenda?: ReferendaProp) { - const { t } = useTranslation(); +export function getNotificationItemTitle (t: TFunction, type: NotificationType, referenda?: ReferendaProp) {
🧹 Nitpick comments (1)
packages/extension-polkagate/src/components/ActionCard.tsx (1)
50-50: Consider using 'block' instead of 'initial' for display.Using
'initial'for the display property may be less predictable than explicitly setting'block'. Since this is a div element,'block'would be more explicit and clearer in intent.Apply this diff if you prefer more explicit styling:
- display: showColorBall ? 'initial' : 'none', + display: showColorBall ? 'block' : 'none',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/extension-polkagate/src/components/ActionButton.tsx(4 hunks)packages/extension-polkagate/src/components/ActionCard.tsx(3 hunks)packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/notification/partials/SelectAccount.tsx(1 hunks)packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx(1 hunks)packages/extension-polkagate/src/hooks/useNotifications.ts(1 hunks)packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/constant.ts(1 hunks)packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts(1 hunks)packages/extension-polkagate/src/popup/notification/partials/Partial.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/util.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
packages/extension-polkagate/src/hooks/useNotifications.ts (10)
packages/extension-polkagate/src/popup/notification/types.ts (2)
NotificationsType(146-153)NotificationActionType(155-161)packages/extension-polkagate/src/popup/notification/util.ts (6)
markMessagesAsRead(221-228)generateReferendaNotifications(87-142)updateReferendas(236-291)generateReceivedFundNotifications(185-214)generateStakingRewardNotifications(149-178)groupNotificationsByDay(305-365)packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (1)
useNotificationSettings(65-197)packages/extension-polkagate/src/hooks/useSelectedChains.ts (1)
useSelectedChains(14-31)packages/extension-polkagate/src/hooks/useGenesisHashOptions.ts (1)
useGenesisHashOptions(14-63)packages/extension-polkagate/src/util/types.ts (1)
DropdownOption(877-880)packages/extension-polkagate/src/util/constants.ts (1)
STORAGE_KEY(310-325)packages/extension-polkagate/src/popup/notification/constant.ts (2)
AUTO_MARK_AS_READ_DELAY(30-30)SUBSCAN_SUPPORTED_CHAINS(51-79)packages/extension-polkagate/src/util/chain.ts (1)
sanitizeChainName(7-26)packages/extension-polkagate/src/popup/notification/helpers.ts (3)
getReceivedFundsInformation(111-179)getPayoutsInformation(187-266)getReferendasInformation(273-336)
packages/extension-polkagate/src/popup/notification/util.ts (5)
packages/extension-polkagate/src/popup/notification/types.ts (6)
ReferendaInformation(121-124)NotificationMessageType(135-144)StakingRewardInformation(115-119)ReceivedFundInformation(109-113)NotificationType(133-133)ReferendaProp(96-107)packages/extension-polkagate/src/popup/notification/constant.ts (1)
NOTIFICATION_TIMESTAMP_OFFSET(18-18)packages/extension-polkagate/src/hooks/useChainInfo.ts (1)
ChainInfo(25-32)packages/extension-polkagate/src/hooks/useTokenPriceBySymbol.ts (1)
Price(17-20)packages/extension-polkagate/src/fullscreen/home/partials/type.ts (1)
CurrencyItemType(4-9)
packages/extension-polkagate/src/fullscreen/notification/partials/SelectAccount.tsx (3)
packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/components/contexts.tsx (1)
AccountContext(34-34)packages/extension-polkagate/src/popup/notification/constant.ts (1)
MAX_ACCOUNT_COUNT_NOTIFICATION(20-20)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx (6)
packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (2)
NotificationSettingType(12-18)useNotificationSettings(65-197)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/fullscreen/home/SelectAccount.tsx (1)
SelectAccount(104-146)packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx (1)
SelectChain(20-71)packages/extension-polkagate/src/popup/notification/constant.ts (2)
SUPPORTED_GOVERNANCE_NOTIFICATION_CHAIN(48-48)SUPPORTED_STAKING_NOTIFICATION_CHAIN(49-49)packages/extension-polkagate/src/fullscreen/components/DraggableModal.tsx (1)
DraggableModal(30-172)
packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (2)
packages/extension-polkagate/src/util/constants.ts (1)
STORAGE_KEY(310-325)packages/extension-polkagate/src/popup/notification/constant.ts (3)
DEFAULT_NOTIFICATION_SETTING(32-38)MAX_ACCOUNT_COUNT_NOTIFICATION(20-20)SET_UP_NOTIFICATION_SETTING(40-46)
packages/extension-polkagate/src/popup/notification/constant.ts (1)
packages/extension-polkagate/src/util/constants.ts (4)
POLKADOT_GENESIS_HASH(58-58)KUSAMA_GENESIS_HASH(59-59)WESTEND_GENESIS_HASH(60-60)PASEO_GENESIS_HASH(61-61)
packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx (3)
packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx (1)
TextValuePair(21-24)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/util/chain.ts (1)
sanitizeChainName(7-26)
packages/extension-polkagate/src/popup/notification/partials/Partial.tsx (3)
packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/components/ActionButton.tsx (1)
ActionButton(65-174)packages/extension-polkagate/src/util/constants.ts (1)
EXTENSION_NAME(6-6)
🔇 Additional comments (3)
packages/extension-polkagate/src/components/ActionCard.tsx (3)
22-24: LGTM! Well-designed prop additions.The new optional props enhance component flexibility while maintaining backward compatibility through proper typing and default values.
27-27: LGTM! Function signature properly updated.The signature correctly includes all new props with sensible defaults (
showChevron = true,showColorBall = true), preserving existing behavior for components that don't pass these props.
94-94: LGTM! Layout changes support flexible card content.The Grid
xsprop enables horizontal expansion, conditional chevron rendering provides UI flexibility, and children placement after the main content allows for extended card content—all aligned with the notification UI requirements.Also applies to: 99-99, 105-105
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
packages/extension-polkagate/src/fullscreen/components/layout/Notifications.tsx (1)
63-69: Unread badge predicate is correct nowThe unread check is fixed to use !read. LGTM.
packages/extension-polkagate/src/hooks/useNotifications.ts (1)
140-172: Persistence after reducer updates: LGTMMoving saves into a useEffect with a queue avoids stale state writes. This resolves the earlier persistence bug.
packages/extension-polkagate/src/popup/notification/helpers.ts (2)
94-97: Normalize referenda status strings (e.g., “TimedOut” → “timeout”)Lowercasing alone yields “timedout”, which doesn’t match “timeout”. Normalize before casting.
- status: referenda.status.toLowerCase() as ReferendaStatus, + status: normalizeReferendaStatus(referenda.status),Add this helper near the top of the file:
const normalizeReferendaStatus = (status: string): ReferendaStatus => { const s = status.replace(/[_\s-]/g, '').toLowerCase(); if (s === 'timedout') return 'timeout'; // Fallback to allowed set; unknown → 'rejected' return (['ongoing','approved','timeout','rejected','cancelled'].includes(s) ? s : 'rejected') as ReferendaStatus; };
179-182: Fix batching cooldown: compare against networks, not addressesSleep condition should check remaining network batches; using addresses.length throttles incorrectly.
- if (i + BATCH_SIZE < addresses.length) { + if (i + BATCH_SIZE < networks.length) { await new Promise((resolve) => setTimeout(resolve, 1000)); }Apply at both locations shown.
Also applies to: 267-269
packages/extension-polkagate/src/popup/notification/util.ts (3)
150-180: Emit a notification for every qualifying staking payoutUsing find limits to one payout per account; iterate all >= latestLoggedIn.
- const newPayouts = payouts.map(({ address, data, network }) => { - const payout = data.find(({ timestamp }) => timestamp >= (latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET)); - return payout ? { address, network, payout } : undefined; - }).filter((item) => !!item); - - newPayouts.forEach(({ address, network, payout }) => { - newMessages.push({ - chain: network, - forAccount: address, - payout, - read: false, - type: 'stakingReward' - }); - }); + payouts.forEach(({ address, data, network }) => { + data + .filter(({ timestamp }) => timestamp >= (latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET)) + .forEach((payout) => { + newMessages.push({ + chain: network, + forAccount: address, + payout, + read: false, + type: 'stakingReward' + }); + }); + });
187-216: Emit a notification for every qualifying received fundSame issue: find emits only one; iterate all transfers since last login.
- const newReceivedFunds = transfers.map(({ address, data, network }) => { - const receivedFund = data.find(({ timestamp }) => timestamp >= (latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET)); - return receivedFund ? { address, network, receivedFund } : undefined; - }).filter((item) => !!item); - - newReceivedFunds.forEach(({ address, network, receivedFund }) => { - newMessages.push({ - chain: network, - forAccount: address, - read: false, - receivedFund, - type: 'receivedFund' - }); - }); + transfers.forEach(({ address, data, network }) => { + data + .filter(({ timestamp }) => timestamp >= (latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET)) + .forEach((receivedFund) => { + newMessages.push({ + chain: network, + forAccount: address, + read: false, + receivedFund, + type: 'receivedFund' + }); + }); + });
498-527: Don’t call hooks in utilities; pass t from the calleruseTranslation inside a plain function violates rules-of-hooks; this will fail lint/build. Pass t as a parameter.
-export function getNotificationItemTitle (type: NotificationType, referenda?: ReferendaProp) { - const { t } = useTranslation(); +export function getNotificationItemTitle (t: TFunction, type: NotificationType, referenda?: ReferendaProp) { switch (type) {Update call sites to pass t obtained in a component/hook.
🧹 Nitpick comments (3)
packages/extension-polkagate/src/hooks/useNotifications.ts (1)
241-261: Consider loading from storage even when notifications are offBlocking LOAD_FROM_STORAGE when notificationIsOff prevents showing old items/status. Suggest drop that guard for load-only.
Apply this diff:
- useEffect(() => { - if (notificationIsOff || initializedRef.current) { + useEffect(() => { + if (initializedRef.current) { return; }packages/extension-polkagate/src/popup/notification/util.ts (2)
533-545: Scale received fund amount with chain decimalsAmounts from Subscan are integer strings; use chainInfo.decimal to scale for display, like staking payouts.
- const assetAmount = formatNumber(item.receivedFund?.amount); + const assetAmount = formatNumber(item.receivedFund?.amount, 2, decimal);
320-337: Deduplicate independent of read flag to avoid duplicates post mark-as-readIncluding read in uniqueKey can show duplicates of the same event before/after mark-as-read.
- uniqueKey = `${item.read}-${JSON.stringify(item.payout ?? '')}`; + uniqueKey = JSON.stringify(item.payout ?? ''); @@ - uniqueKey = `${item.read}-${JSON.stringify(item.receivedFund ?? '')}`; + uniqueKey = JSON.stringify(item.receivedFund ?? ''); @@ - uniqueKey = `${item.read}-${JSON.stringify(item.referenda ?? '')}`; + uniqueKey = JSON.stringify(item.referenda ?? '');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/extension-polkagate/src/fullscreen/components/layout/Notifications.tsx(1 hunks)packages/extension-polkagate/src/hooks/useNotifications.ts(1 hunks)packages/extension-polkagate/src/popup/notification/helpers.ts(1 hunks)packages/extension-polkagate/src/popup/notification/util.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/extension-polkagate/src/fullscreen/components/layout/Notifications.tsx (3)
packages/extension-polkagate/src/hooks/useIsExtensionPopup.ts (1)
useIsExtensionPopup(4-21)packages/extension-polkagate/src/util/handleExtensionPopup.ts (1)
useExtensionPopups(11-18)packages/extension-polkagate/src/hooks/useNotifications.ts (1)
useNotifications(107-308)
packages/extension-polkagate/src/popup/notification/helpers.ts (10)
packages/extension-polkagate/src/popup/notification/types.ts (10)
TransferSubscan(14-38)TransfersProp(78-87)PayoutSubscan(40-50)PayoutsProp(89-94)ReferendaSubscan(52-62)ReferendaProp(96-107)ReceivedFundInformation(109-113)ApiResponse(7-12)StakingRewardInformation(115-119)ReferendaInformation(121-124)packages/extension-polkagate/src/util/types.ts (1)
DropdownOption(877-880)packages/extension-polkagate/src/util/address.ts (1)
getSubstrateAddress(40-59)packages/extension-polkagate/src/popup/notification/util.ts (1)
timestampToDate(18-53)packages/extension-polkagate/src/popup/notification/constant.ts (5)
ReferendaStatus(6-6)BATCH_SIZE(17-17)MAX_RETRIES(16-16)REFERENDA_COUNT_TO_TRACK_DOT(10-10)REFERENDA_COUNT_TO_TRACK_KSM(11-11)packages/extension-polkagate/src/util/migrateHubUtils.ts (1)
isMigratedRelay(103-105)packages/extension-polkagate/src/util/getChainName.ts (1)
getChainName(15-29)packages/extension-polkagate/src/util/chain.ts (1)
getSubscanChainName(32-54)packages/extension-polkagate/src/util/api/postData.ts (1)
postData(6-25)packages/extension-polkagate/src/util/constants.ts (2)
POLKADOT_GENESIS_HASH(58-58)KUSAMA_GENESIS_HASH(59-59)
packages/extension-polkagate/src/hooks/useNotifications.ts (10)
packages/extension-polkagate/src/popup/notification/types.ts (2)
NotificationsType(146-153)NotificationActionType(155-161)packages/extension-polkagate/src/popup/notification/util.ts (6)
markMessagesAsRead(223-230)generateReferendaNotifications(88-143)updateReferendas(238-293)generateReceivedFundNotifications(187-216)generateStakingRewardNotifications(150-180)groupNotificationsByDay(307-389)packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (1)
useNotificationSettings(65-197)packages/extension-polkagate/src/hooks/useSelectedChains.ts (1)
useSelectedChains(14-31)packages/extension-polkagate/src/hooks/useGenesisHashOptions.ts (1)
useGenesisHashOptions(14-63)packages/extension-polkagate/src/util/types.ts (1)
DropdownOption(877-880)packages/extension-polkagate/src/util/constants.ts (1)
STORAGE_KEY(310-325)packages/extension-polkagate/src/popup/notification/constant.ts (2)
AUTO_MARK_AS_READ_DELAY(30-30)SUBSCAN_SUPPORTED_CHAINS(51-79)packages/extension-polkagate/src/util/chain.ts (1)
sanitizeChainName(7-26)packages/extension-polkagate/src/popup/notification/helpers.ts (3)
getReceivedFundsInformation(112-187)getPayoutsInformation(195-274)getReferendasInformation(281-344)
packages/extension-polkagate/src/popup/notification/util.ts (6)
packages/extension-polkagate/src/popup/notification/types.ts (6)
ReferendaInformation(121-124)NotificationMessageType(135-144)StakingRewardInformation(115-119)ReceivedFundInformation(109-113)NotificationType(133-133)ReferendaProp(96-107)packages/extension-polkagate/src/popup/notification/constant.ts (1)
NOTIFICATION_TIMESTAMP_OFFSET(18-18)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/hooks/useChainInfo.ts (1)
ChainInfo(25-32)packages/extension-polkagate/src/hooks/useTokenPriceBySymbol.ts (1)
Price(17-20)packages/extension-polkagate/src/fullscreen/home/partials/type.ts (1)
CurrencyItemType(4-9)
🔇 Additional comments (1)
packages/extension-polkagate/src/fullscreen/components/layout/Notifications.tsx (1)
58-79: react-router-dom v6+ confirmed react-router-dom ^7.4.1 is used in extension-polkagate and extension-ui, so useNavigate is supported.
packages/extension-polkagate/src/fullscreen/components/layout/Notifications.tsx
Show resolved
Hide resolved
|
please merghe main into the branch and |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/extension-polkagate/src/popup/notification/util.ts (2)
148-178: Emit a notification for every new staking payout (not just the first).Using Array.find captures only a single payout per account; additional new rewards are dropped. Iterate all qualifying payouts.
export const generateStakingRewardNotifications = ( latestLoggedIn: number, payouts: StakingRewardInformation[] ): NotificationMessageType[] => { const newMessages: NotificationMessageType[] = []; - - const newPayouts = payouts.map(({ address, data, network }) => { - const payout = data.find(({ timestamp }) => timestamp >= (latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET)); - - return payout - ? { - address, - network, - payout - } - : undefined; - }).filter((item) => !!item); - - // Generate notifications for new payouts - newPayouts.forEach(({ address, network, payout }) => { - newMessages.push({ - chain: network, - forAccount: address, - payout, - read: false, - type: 'stakingReward' - }); - }); + payouts.forEach(({ address, data, network }) => { + data + .filter(({ timestamp }) => timestamp >= (latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET)) + .forEach((payout) => { + newMessages.push({ + chain: network, + forAccount: address, + payout, + read: false, + type: 'stakingReward' + }); + }); + }); return newMessages; };
185-214: Emit a notification for every received fund event.Same issue here: Array.find limits you to one transfer per account; others are missed. Collect all qualifying transfers.
export const generateReceivedFundNotifications = ( latestLoggedIn: number, transfers: ReceivedFundInformation[] ): NotificationMessageType[] => { const newMessages: NotificationMessageType[] = []; - const newReceivedFunds = transfers.map(({ address, data, network }) => { - const receivedFund = data.find(({ timestamp }) => timestamp >= (latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET)); - - return receivedFund - ? { - address, - network, - receivedFund - } - : undefined; - }).filter((item) => !!item); - - // Generate notifications for new payouts - newReceivedFunds.forEach(({ address, network, receivedFund }) => { - newMessages.push({ - chain: network, - forAccount: address, - read: false, - receivedFund, - type: 'receivedFund' - }); - }); + transfers.forEach(({ address, data, network }) => { + data + .filter(({ timestamp }) => timestamp >= (latestLoggedIn - NOTIFICATION_TIMESTAMP_OFFSET)) + .forEach((receivedFund) => { + newMessages.push({ + chain: network, + forAccount: address, + read: false, + receivedFund, + type: 'receivedFund' + }); + }); + }); return newMessages; };packages/extension-polkagate/src/popup/notification/index.tsx (1)
21-21: Allow live fetch: pass false to useNotifications.Without an argument, justLoadData stays true and the hook won’t fetch from network on first load. Pass false.
- const { markAsRead, notificationItems, status } = useNotifications(); + const { markAsRead, notificationItems, status } = useNotifications(false);packages/extension-polkagate/src/popup/notification/partials/NotificationGroup.tsx (1)
68-70: Fix milliseconds vs seconds fallback in getTimeOfDay.getTimeOfDay expects seconds; Date.now() is milliseconds. Convert the fallback.
- const time = getTimeOfDay(item.payout?.timestamp ?? item.receivedFund?.timestamp ?? item.referenda?.latestTimestamp ?? Date.now()); + const fallbackTimestamp = Math.floor(Date.now() / 1000); + const time = getTimeOfDay(item.payout?.timestamp ?? item.receivedFund?.timestamp ?? item.referenda?.latestTimestamp ?? fallbackTimestamp);
🧹 Nitpick comments (10)
packages/extension-polkagate/src/popup/notification/util.ts (6)
374-381: Avoid parsing formatted dates for sorting.Sorting by new Date("15 Dec 2025") depends on locale parsing and can be brittle. Keep a numeric “day start” (e.g., UTC midnight seconds) alongside the label and sort by that.
Example approach:
- Compute dayStart = new Date(Date.UTC(y, m, d)).getTime()
- Store grouped as Map<number, {label: string, items: ...}>
- Sort by numeric key, then render label.
439-494: Make decimal scaling robust for numeric inputs.When decimal > 0 and value is a number, the BigInt branch is skipped, risking precision loss for large chain amounts. Coerce to an integer string first, then use BigInt.
- const strValue = value.toString().replace(/,/g, '').trim(); + const strValue = value.toString().replace(/,/g, '').trim(); @@ - // If blockchain-style decimal division is required - if (decimal > 0 && /^[0-9]+$/.test(strValue)) { + // If blockchain-style decimal division is required + if (decimal > 0) { + // drop any fractional part before BigInt conversion + const intLike = strValue.split('.')[0]; + if (!/^[0-9]+$/.test(intLike)) { + return 0; + } // Use BigInt division for safety - const bigintVal = BigInt(strValue); + const bigintVal = BigInt(intLike);
507-513: Fix wording: “Referendum timed out”.“time outed” reads incorrectly.
- } else if (referenda?.status === 'timeout') { - return t('Referendum time outed'); + } else if (referenda?.status === 'timeout') { + return t('Referendum timed out');Remember to align the translation key. See translation.json comment.
525-536: Guard missing currency sign.When currency is undefined,
${currencySign}${amount}renders “undefined123”. Default to empty string.- const currencySign = currency?.sign; + const currencySign = currency?.sign ?? '';
590-601: Add icon styles for ‘submitted’ and ‘decision’ referendum statuses.These fall back to the neutral “rejected” style today. Provide explicit mappings for visual consistency.
const statusMap = { approved: { ItemIcon: Receipt2, bgcolor: '#FF4FB91A', borderColor: '#FF4FB940', color: '#FF4FB9' }, cancelled: neutralStyle, + decision: { ItemIcon: Receipt2, bgcolor: '#82FFA540', borderColor: '#82FFA51A', color: '#82FFA5' }, ongoing: { ItemIcon: Receipt2, bgcolor: '#82FFA540', borderColor: '#82FFA51A', color: '#82FFA5' }, rejected: neutralStyle, + submitted: { ItemIcon: Receipt2, bgcolor: '#82FFA540', borderColor: '#82FFA51A', color: '#82FFA5' }, timeout: neutralStyle };
236-241: Fix naming typos: "updateReferendas" and "preciousRefs".Rename to
updateReferendaandpreviousRefsfor consistency with theReferendaInformationtype and to fix the apparent spelling error. One call site requires updating atpackages/extension-polkagate/src/hooks/useNotifications.ts:55.-export const updateReferendas = (preciousRefs: ReferendaInformation[] | null | undefined, newRefs: ReferendaInformation[]) => { - if (!preciousRefs) { +export const updateReferenda = (previousRefs: ReferendaInformation[] | null | undefined, newRefs: ReferendaInformation[]) => { + if (!previousRefs) { return newRefs; } const resultMap = new Map<string | number, ReferendaInformation>(); // Copy all previous data - for (const prev of preciousRefs) { + for (const prev of previousRefs) {Update the call site in
packages/extension-polkagate/src/hooks/useNotifications.ts:55:- referendas: updateReferendas(state.referendas, action.payload) + referendas: updateReferenda(state.referendas, action.payload)packages/extension/public/locales/en/translation.json (3)
819-819: Wording: “Enable Received Funds”.“Enable Receive Fund” is ungrammatical. Suggest “Enable Received Funds” (or “Enable Incoming Funds”).
- "Enable Receive Fund": "", + "Enable Received Funds": "",
824-824: Clarify incomplete sentence.“Select up to 3 accounts to be notified when account activity” → add missing verb.
- "Select up to 3 accounts to be notified when account activity": "", + "Select up to 3 accounts to be notified of account activity": "",
835-835: Fix wording: “Referendum timed out”.Align with util.ts change.
- "Referendum time outed": "", + "Referendum timed out": "",packages/extension-polkagate/src/popup/notification/partials/NotificationGroup.tsx (1)
68-73: Graceful chain defaults in descriptions.When chainInfo is unresolved (e.g., missing genesisHash), description interpolations can render “undefined”. Consider falling back to item.chain?.text.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/extension-polkagate/src/popup/notification/index.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/partials/NotificationGroup.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/util.ts(1 hunks)packages/extension/package.json(1 hunks)packages/extension/public/locales/en/translation.json(1 hunks)packages/extension/src/packageInfo.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/extension/src/packageInfo.ts
- packages/extension/package.json
🧰 Additional context used
🧬 Code graph analysis (3)
packages/extension-polkagate/src/popup/notification/index.tsx (2)
packages/extension-polkagate/src/hooks/useNotifications.ts (1)
useNotifications(107-308)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)
packages/extension-polkagate/src/popup/notification/util.ts (5)
packages/extension-polkagate/src/popup/notification/types.ts (6)
ReferendaInformation(121-124)NotificationMessageType(135-144)StakingRewardInformation(115-119)ReceivedFundInformation(109-113)NotificationType(133-133)ReferendaProp(96-107)packages/extension-polkagate/src/popup/notification/constant.ts (1)
NOTIFICATION_TIMESTAMP_OFFSET(18-18)packages/extension-polkagate/src/hooks/useChainInfo.ts (1)
ChainInfo(25-32)packages/extension-polkagate/src/hooks/useTokenPriceBySymbol.ts (1)
Price(17-20)packages/extension-polkagate/src/fullscreen/home/partials/type.ts (1)
CurrencyItemType(4-9)
packages/extension-polkagate/src/popup/notification/partials/NotificationGroup.tsx (7)
packages/extension-polkagate/src/popup/notification/util.ts (5)
isToday(396-405)getNotificationItemTitle(496-523)getTimeOfDay(413-423)getNotificationDescription(525-580)getNotificationIcon(582-606)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/hooks/useAccount.ts (1)
useAccount(12-28)packages/extension-polkagate/src/util/address.ts (1)
toShortAddress(61-69)packages/extension-polkagate/src/popup/notification/types.ts (1)
NotificationMessageType(135-144)packages/extension-polkagate/src/hooks/useChainInfo.ts (1)
useChainInfo(42-73)packages/extension-polkagate/src/hooks/useTokenPriceBySymbol.ts (1)
useTokenPriceBySymbol(31-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/extension/public/locales/en/translation.json (1)
1-848: i18n init already prevents empty-string UI blanks — no blocking change requiredVerified packages/extension-polkagate/src/i18n/i18n.ts sets returnEmptyString: false and fallbackLng: 'en', so empty values in packages/extension/public/locales/en/translation.json will not render blank UI; populate the en file if you want readable English copy.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx (4)
10-11: Consider importing from shared/centralized paths.Importing from
popup/notification/...paths in a fullscreen component creates architectural coupling. Based on the PR context, these hooks and constants should be imported from shared locations (e.g.,@polkadot/extension-polkagate/src/hooksfor the hook).Apply this diff:
-import { useTranslation } from '@polkadot/extension-polkagate/src/hooks'; -import { SUPPORTED_GOVERNANCE_NOTIFICATION_CHAIN, SUPPORTED_STAKING_NOTIFICATION_CHAIN } from '@polkadot/extension-polkagate/src/popup/notification/constant'; -import useNotificationSettings, { type NotificationSettingType, Popups } from '@polkadot/extension-polkagate/src/popup/notification/hook/useNotificationSettings'; +import { useNotificationSettings, useTranslation } from '@polkadot/extension-polkagate/src/hooks'; +import { SUPPORTED_GOVERNANCE_NOTIFICATION_CHAIN, SUPPORTED_STAKING_NOTIFICATION_CHAIN } from '@polkadot/extension-polkagate/src/popup/notification/constant'; +import { type NotificationSettingType, Popups } from '@polkadot/extension-polkagate/src/popup/notification/hook/useNotificationSettings';
76-93: Consider adding count badges for Governance and Staking Rewards.The Accounts card displays a count badge (lines 72-74), but Governance and Staking Rewards cards don't show how many chains are selected. This creates an inconsistent UX—users would benefit from seeing the selection count at a glance.
Apply this diff for Governance:
<ActionCard Icon={Record} iconColor='#FF4FB9' iconSize={24} iconWithoutTransform onClick={openPopup(Popups.GOVERNANCE)} style={{ ...CARD_STYLE }} title={t('Governance')} - /> + > + <Typography color='#AA83DC' sx={{ bgcolor: '#BFA1FF26', borderRadius: '10px', mr: '2px', p: '3px 10px' }} variant='B-3'> + {notificationSetting.governance?.length} + </Typography> + </ActionCard>And for Staking Rewards:
<ActionCard Icon={BuyCrypto} iconColor='#FF4FB9' iconSize={24} iconWithoutTransform onClick={openPopup(Popups.STAKING_REWARDS)} style={{ ...CARD_STYLE }} title={t('Staking Rewards')} - /> + > + <Typography color='#AA83DC' sx={{ bgcolor: '#BFA1FF26', borderRadius: '10px', mr: '2px', p: '3px 10px' }} variant='B-3'> + {notificationSetting.stakingRewards?.length} + </Typography> + </ActionCard>
115-149: Add a default case for type safety.The switch statement handling
popupslacks a default case. While all currentPopupsenum values are covered, adding a default case improves type safety and maintainability if new popup types are added later.Apply this diff:
case Popups.STAKING_REWARDS: return ( <SelectChain onChains={setStakingRewardsChains} options={SUPPORTED_STAKING_NOTIFICATION_CHAIN} previousState={notificationSetting.stakingRewards} />); + + default: + return null; } }, [notificationSetting, openPopup, popups, setAccounts, setGovernanceChains, setStakingRewardsChains, toggleNotification, toggleReceivedFunds]);
184-184: Remove extra space in padding value.The padding string has an unnecessary leading space:
' 20px 10px 10px'. While this may still work, it's cleaner to remove it.Apply this diff:
- style={{ backgroundColor: '#1B133C', minHeight: '600px', overflow: 'clip', padding: ' 20px 10px 10px' }} + style={{ backgroundColor: '#1B133C', minHeight: '600px', overflow: 'clip', padding: '20px 10px 10px' }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx (4)
packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (2)
NotificationSettingType(12-18)useNotificationSettings(65-197)packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx (1)
SelectChain(20-71)packages/extension-polkagate/src/popup/notification/constant.ts (2)
SUPPORTED_GOVERNANCE_NOTIFICATION_CHAIN(48-48)SUPPORTED_STAKING_NOTIFICATION_CHAIN(49-49)packages/extension-polkagate/src/fullscreen/components/DraggableModal.tsx (1)
DraggableModal(30-172)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx (2)
76-93: Consider showing selection counts for consistency.The Accounts card (lines 72-74) displays a count badge, but the Governance and Staking Rewards cards don't show how many chains are selected. Adding similar count badges would improve consistency and user feedback.
Example for Governance card:
<ActionCard Icon={Record} iconColor='#FF4FB9' iconSize={24} iconWithoutTransform onClick={openPopup(Popups.GOVERNANCE)} style={{ ...CARD_STYLE }} title={t('Governance')} - /> + > + <Typography color='#AA83DC' sx={{ bgcolor: '#BFA1FF26', borderRadius: '10px', mr: '2px', p: '3px 10px' }} variant='B-3'> + {notificationSetting.governance?.length ?? 0} + </Typography> + </ActionCard>Apply similar changes to the Staking Rewards card.
115-149: Consider adding a default case for defensive coding.The switch statement covers all current Popups enum values but lacks a default case. If the enum is extended or an unexpected value occurs,
uiwill be undefined, potentially causing render issues.Apply this diff:
case Popups.STAKING_REWARDS: return ( <SelectChain onChains={setStakingRewardsChains} options={SUPPORTED_STAKING_NOTIFICATION_CHAIN} previousState={notificationSetting.stakingRewards} />); + + default: + return ( + <SettingUI + notificationSetting={notificationSetting} + openPopup={openPopup} + toggleNotification={toggleNotification} + toggleReceivedFunds={toggleReceivedFunds} + />); } }, [notificationSetting, openPopup, popups, setAccounts, setGovernanceChains, setStakingRewardsChains, toggleNotification, toggleReceivedFunds]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx(1 hunks)packages/extension-polkagate/src/popup/notification/util.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/extension-polkagate/src/popup/notification/NotificationSettings.tsx
- packages/extension-polkagate/src/popup/notification/util.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx (5)
packages/extension-polkagate/src/popup/notification/hook/useNotificationSettings.ts (2)
NotificationSettingType(12-18)useNotificationSettings(65-197)packages/extension-polkagate/src/components/translate.ts (1)
useTranslation(8-10)packages/extension-polkagate/src/fullscreen/notification/partials/SelectChain.tsx (1)
SelectChain(20-71)packages/extension-polkagate/src/popup/notification/constant.ts (2)
SUPPORTED_GOVERNANCE_NOTIFICATION_CHAIN(48-48)SUPPORTED_STAKING_NOTIFICATION_CHAIN(49-49)packages/extension-polkagate/src/fullscreen/components/DraggableModal.tsx (1)
DraggableModal(30-172)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
🔇 Additional comments (3)
packages/extension-polkagate/src/fullscreen/notification/NotificationSettingsFS.tsx (3)
1-17: LGTM!Imports are well-organized and all are utilized. The CARD_STYLE constant appropriately centralizes shared styling.
56-56: Past review comment appears resolved.The translation key now correctly uses "Enable Received Funds" as suggested in the previous review.
179-190: LGTM!The DraggableModal integration is clean, with appropriate props and styling. The component structure follows React best practices with proper hook usage and TypeScript typing.
…old instead of hard coded
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/extension-polkagate/src/popup/notification/helpers.ts (4)
24-28: Normalize both addresses before comparison.The filter on Line 26 compares
getSubstrateAddress(transfer.to)(normalized to SS58-42) against the rawaddressparameter, which may be in a different SS58 format. This asymmetry causes missed matches whenaddressisn't already SS58-42.Apply this diff to normalize both sides:
+ const normalizedAddress = getSubstrateAddress(address); + const result = transfers.reduce((accumulator, transfer) => { - if (getSubstrateAddress(transfer.to) !== address) { + if (getSubstrateAddress(transfer.to) !== normalizedAddress) { return accumulator; }
94-94: Status normalization doesn't handle camelCase variants.Simply lowercasing
referenda.statuson Line 94 won't properly map Subscan's"TimedOut"to the expected'timedOut'union member—it produces'timedout'instead, causing the status branch to fail.Apply this diff to normalize correctly:
+const normalizeReferendaStatus = (status: string): ReferendaStatus => { + const normalized = status.replace(/_/g, '').toLowerCase(); + + return normalized === 'timedout' + ? 'timedOut' + : normalized as ReferendaStatus; +}; + const transformReferendas = (referendas: ReferendaSubscan[], network: DropdownOption) => { ... const result = filtered.reduce((accumulator, referenda) => { const sanitizedReferenda = { ... - status: referenda.status.toLowerCase() as ReferendaStatus, + status: normalizeReferendaStatus(referenda.status), ... };
180-182: Fix throttling condition to check networks.length.Line 180 compares
i + BATCH_SIZE < addresses.length, butiindexes into thenetworksarray. This breaks rate limiting: the code may skip necessary pauses when more network batches remain, or pause when none are queued.Apply this diff:
- if (i + BATCH_SIZE < addresses.length) { + if (i + BATCH_SIZE < networks.length) { await new Promise((resolve) => setTimeout(resolve, 1000)); }
267-269: Fix throttling condition to check networks.length.Line 267 compares
i + BATCH_SIZE < addresses.length, butiindexes thenetworksarray. This breaks rate limiting: it may skip necessary pauses when more network batches remain, or pause when none are queued.Apply this diff:
- if (i + BATCH_SIZE < addresses.length) { + if (i + BATCH_SIZE < networks.length) { await new Promise((resolve) => setTimeout(resolve, 1000)); }
🧹 Nitpick comments (1)
packages/extension-polkagate/src/popup/notification/helpers.ts (1)
289-341: Consider adding throttling between network requests.Unlike
getReceivedFundsInformationandgetPayoutsInformation, this function processes networks sequentially without any delay between requests. Adding a small delay (e.g., 500-1000ms) would provide consistent rate limiting and reduce the risk of hitting Subscan's throttles.For example, add a delay after each successful network request:
for (const network of networks) { ... const result = await promise(); if (result) { results.push(result); } + + // Optional: Add delay between networks for rate limiting + await new Promise((resolve) => setTimeout(resolve, 500)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/extension-polkagate/src/popup/notification/constant.ts(1 hunks)packages/extension-polkagate/src/popup/notification/helpers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/popup/notification/constant.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/extension-polkagate/src/popup/notification/helpers.ts (10)
packages/extension-polkagate/src/popup/notification/types.ts (10)
TransferSubscan(14-38)TransfersProp(78-87)PayoutSubscan(40-50)PayoutsProp(89-94)ReferendaSubscan(52-62)ReferendaProp(96-107)ReceivedFundInformation(109-113)ApiResponse(7-12)StakingRewardInformation(115-119)ReferendaInformation(121-124)packages/extension-polkagate/src/util/types.ts (1)
DropdownOption(877-880)packages/extension-polkagate/src/util/address.ts (1)
getSubstrateAddress(40-59)packages/extension-polkagate/src/popup/notification/util.ts (1)
timestampToDate(16-51)packages/extension-polkagate/src/popup/notification/constant.ts (7)
ReferendaStatus(6-6)BATCH_SIZE(19-19)MAX_RETRIES(18-18)RECEIVED_FUNDS_THRESHOLD(10-10)RECEIVED_REWARDS_THRESHOLD(11-11)REFERENDA_COUNT_TO_TRACK_DOT(12-12)REFERENDA_COUNT_TO_TRACK_KSM(13-13)packages/extension-polkagate/src/util/migrateHubUtils.ts (1)
isMigratedRelay(103-105)packages/extension-polkagate/src/util/getChainName.ts (1)
getChainName(15-29)packages/extension-polkagate/src/util/chain.ts (1)
getSubscanChainName(32-54)packages/extension-polkagate/src/util/api/postData.ts (1)
postData(6-25)packages/extension-polkagate/src/util/constants.ts (2)
POLKADOT_GENESIS_HASH(58-58)KUSAMA_GENESIS_HASH(59-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
| const payoutInfo = [...(soloPayoutInfo?.data?.list ?? []), ...(poolPayoutInfo?.data?.list ?? [])]; | ||
|
|
||
| if (!payoutInfo) { | ||
| return null; // account doesn't have any history | ||
| } |
There was a problem hiding this comment.
Remove unreachable null check.
Line 236 constructs payoutInfo by spreading two arrays, which always yields an array (possibly empty []), never null or undefined. The check on Line 238 is unreachable and should be removed.
Apply this diff:
const payoutInfo = [...(soloPayoutInfo?.data?.list ?? []), ...(poolPayoutInfo?.data?.list ?? [])];
-if (!payoutInfo) {
- return null; // account doesn't have any history
-}
+if (payoutInfo.length === 0) {
+ return null; // account doesn't have any history
+}
return transformPayouts(address, payoutInfo, network);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const payoutInfo = [...(soloPayoutInfo?.data?.list ?? []), ...(poolPayoutInfo?.data?.list ?? [])]; | |
| if (!payoutInfo) { | |
| return null; // account doesn't have any history | |
| } | |
| const payoutInfo = [...(soloPayoutInfo?.data?.list ?? []), ...(poolPayoutInfo?.data?.list ?? [])]; | |
| if (payoutInfo.length === 0) { | |
| return null; // account doesn't have any history | |
| } |
🤖 Prompt for AI Agents
In packages/extension-polkagate/src/popup/notification/helpers.ts around lines
236 to 240, the code builds payoutInfo by spreading two lists which always
produces an array (never null/undefined), so the subsequent null check is
unreachable; remove the `if (!payoutInfo) { return null }` block and any
dependent branch assuming payoutInfo can be null, leaving logic to handle an
empty array instead.
2 participants
Summary by CodeRabbit
New Features
Style
Chores