Update modal architecture#14223
Open
dylanjeffers wants to merge 13 commits intomainfrom
Open
Conversation
|
Contributor
🌐 Web preview readyPreview URL: https://audius-web-preview-pr-14223.audius.workers.dev Unique preview for this PR (deployed from this branch). |
Establishes two reference patterns to replace the global Redux modal/drawer
registry, which everything else can migrate to incrementally:
1. Colocated (local useState) — for modals opened from a single feature.
Reference: EditFolderModal (web), FeedFilterDrawer (mobile + web mobile).
Removed their slices/registry entries; modal mounts inline next to its
trigger via local state.
2. Registry via @ebay/nice-modal-react — for modals opened from many places,
sagas, deep links, etc. Reference: ShareModal (web) + ShareDrawer (mobile).
- New platform-agnostic bridge in common/services/nice-modal-bridge so
sagas can call showNiceModal('Share') without depending on the React
library directly.
- Web/mobile app roots wire the bridge via setNiceModalAdapter() and
mount <NiceModal.Provider> deep enough in the tree to expose
AudiusQueryProvider/ToastContext/NavigationContainer to NiceModal-
managed modals.
- Bridge saga in share-modal/sagas.ts translates legacy
setVisibility('Share', true) calls into showNiceModal('Share'), so
the existing ~10 trigger sites keep working unchanged.
- registerNiceModals.ts at each app root collects the side-effect
imports for NiceModal.register(...) — add new modals here as they
migrate.
Smoke-tested on web (localhost:3002) and iOS simulator: ShareModal/Drawer
open via natural flow, and a stray clipping issue on FeedFilterDrawer is
fixed by wrapping in <Portal hostName='DrawerPortal'>.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single-trigger modal opened only by LabelAccountSettingsCard (and auto-opened on a route match). Converts to local useState + inline mount, deletes the registry entry / parentSlice initial state / type union member. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the share-modal-specific watchOpenShareViaSetVisibility saga
with a generic bridge in services/nice-modal-bridge that translates
setVisibility(id, true|false) into showNiceModal(id) / hideNiceModal(id)
for any modal id that opts in via registerNiceModalId.
ShareModal (web) and ShareDrawer (mobile) now call
registerNiceModalId('Share') alongside their NiceModal.register(...) so
the existing ~10 trigger sites keep working unchanged.
This makes the per-modal NiceModal migration cost just three things:
1. Wrap with NiceModal.create(...)
2. NiceModal.register('X', Component) + registerNiceModalId('X')
3. Side-effect import in registerNiceModals.ts
4. Remove from web Modals.tsx / mobile Drawers.tsx (avoid double mount)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migrates the simpler global modals to nice-modal-react:
- LeavingAudiusModal
- TierExplainerModal (TiersExplainer)
- WelcomeModal
- DeletePlaylistConfirmationModal
- DeleteTrackConfirmationModal
- DuplicateAddConfirmationModal
- PublishConfirmationModal
- UploadConfirmationModal
- ReplaceTrackConfirmationModal
- ReplaceTrackProgressModal
- EarlyReleaseConfirmationModal
- EditAccessConfirmationModal
- HideContentConfirmationModal
- AlbumTrackRemoveConfirmationModal
- FinalizeWinnersConfirmationModal
Each modal:
- wraps with NiceModal.create()
- reads visibility via useModal() (legacy createModal hook still
provides `data` payloads — that part is unchanged)
- calls NiceModal.register(id, Component) + registerNiceModalId(id)
- removed from web Modals.tsx commonModalsMap to avoid double-mount
- added to registerNiceModals.ts side-effect import list
Trigger sites untouched — the generalized bridge saga translates
existing setVisibility(id, true) dispatches to showNiceModal(id).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- InboxUnavailableModal - ArtistPickModal - PayoutWalletModal - DownloadTrackArchiveModal - TransactionDetailsModal - LockedContentModal - WaitForDownloadModal Same migration shape as batch 1: NiceModal.create wrap, useModal() for visibility, registerNiceModalId, side-effect import in registerNiceModals.ts, removed from web Modals.tsx commonModalsMap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- AudioBreakdownModal - ConnectedWalletsModal - AddCashModal - BuySellModal - ReceiveTokensModal - ChatBlastModal Same migration shape: NiceModal.create wrap, useModal() for visibility, registerNiceModalId, side-effect import, removed from Modals.tsx. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- USDCPurchaseDetailsModal - USDCTransactionDetailsModal - ClaimVestedCoinsModal Same migration shape: NiceModal.create wrap, useModal() for visibility, registerNiceModalId, side-effect import, removed from Modals.tsx. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- TopAPIModal (APIRewardsExplainer) - ClaimAllRewardsModal Same migration shape: NiceModal.create wrap, useModal() for visibility, registerNiceModalId, side-effect import, removed from Modals.tsx. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-fix prettier/import-order issues from the bulk migration; remove unused useChatBlastModal import from ChatBlastModal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Continues the Wave D migration to nice-modal-react for the remaining modals in web Modals.tsx commonModalsMap: - HostRemixContestModal (keeps useHostRemixContestModal data hook) - AddToCollectionModal - ChallengeRewardsModal Same shape as Wave D: NiceModal.create wrap, useModal() for visibility, registerNiceModalId opts each id into the bridge allowlist, side-effect import added to registerNiceModals.ts, removed from Modals.tsx commonModalsMap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two fixes to the Wave D modal bridge that previously left several modals
unable to open:
1. The bridge saga (`niceModalBridgeSagas`) was defined in `common` but
never spawned by web or mobile rootSagas, so even the existing
`setVisibility(id, true) → showNiceModal(id)` translation was dead
code. Add it to both rootSagas and re-export from
`@audius/common/services` so consumers can import it directly.
2. createModal-driven modals (LeavingAudiusModal, ArtistPickModal, etc.)
open via `useFooModal().onOpen()` which dispatches
`modals/{id}/open` — not the parent `setVisibility`. The bridge only
listened for `setVisibility`, so these modals never reached
`showNiceModal`. Add `watchOpenViaCreateModal` that matches the
generic `modals/{id}/(open|close|closed)` action shape and bridges to
`showNiceModal` / `hideNiceModal` when the id is in the registry.
Together this restores Wave D's NiceModal-managed modals and unblocks
migrating the remaining createModal-based modals in subsequent waves.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migrates the remaining createModal-driven modals from Modals.tsx to
nice-modal-react now that the generalized bridge handles
`modals/{id}/open` actions:
- TransferAudioMobileDrawer (TransferAudioMobileWarning)
- WithdrawUSDCModal
- CoinflowOnrampModal (CoinflowOnramp)
- CoinflowWithdrawModal (CoinflowWithdraw)
- SendTokensModal
- PremiumContentPurchaseModal
Same shape as Wave D/E batch 1: NiceModal.create wrap, useModal() for
visibility, registerNiceModalId opts each id into the bridge allowlist,
side-effect import added to registerNiceModals.ts, removed from
Modals.tsx commonModalsMap.
Modals.tsx is now down to the lazy-loaded set (InboxSettings,
CommentSettings, CreateChatModal, StripeOnRamp) plus
BrowserPushPermissionConfirmation; those need a separate plan to
preserve their lazy-load semantics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dylanjeffers
force-pushed
the
colocate-modals-step-1
branch
from
e3c7172 to
c230a9f
Compare
The web preview deploy was hitting Cloudflare's 25 MiB per-asset KV limit on `assets/chunks/chunk-*.js.map`. Release and production deploys already move sourcemaps out of the bundle (and upload them to S3) before the wrangler step; the preview job didn't, so the full maps were being shipped to KV and one chunk's map exceeded the limit. We don't need preview maps anywhere — just delete them from the build output right before wrangler deploys. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Updates modals to render near content as much as possible
For modals that need to render from a trigger outside the ui, replace redux with nice-modal
Update mobile to use bottom-sheet as much as possible