refactor: deduplicate client lookup, section headers, and confirm dialogs#143
Merged
Conversation
The 'accordAuthProvider.select((s) => s is AccordAuthLoggedIn ? s.client : null)' lookup was copy-pasted into ~28 ConsumerState getters across the app. Extract it once as WidgetRef/Ref 'accordClient' extensions in shared/utils/client_access.dart and have each '_client' getter delegate, dropping the now-unused auth imports where they were only used for it.
Four screens (settings, developer, voice, privacy) each declared an identical private _SectionHeader/_Header widget rendering an uppercase labelMedium/gray header. Replace them with a single shared/components/section_header.dart and repoint all call sites.
The Cancel/confirm AlertDialog was duplicated across the three admin tabs, ban list, emoji management, channel management, DM conversations, member popout and privacy settings — some as private _confirm methods, some inline. Replace all with a single shared/utils/confirm_dialog.dart helper (optional danger styling). The few sites that used a plain TextButton or Material red for the confirm action now render the standard FilledButton / themed-red used everywhere else.
…ty tests Three top-level / non-getter call sites were not migrated in the original commit: confirmAndDeleteChannel (channel_management.dart), openAccordDirectMessage (accord_direct_messages.dart), and _ackLast (accord_home.dart). All three now use ref.accordClient. The now-dead accordion auth imports in channel_management.dart are removed. Also adds widget tests for SectionHeader and showConfirmDialog, the two new shared utilities introduced in this PR. https://claude.ai/code/session_015D7sLdyaTejJUuTfZNk4v3
Contributor
Author
Code Review — findings & fixes (commit
|
| File | Location | Problem |
|---|---|---|
lib/features/channels/components/channel_management.dart |
confirmAndDeleteChannel() line 88 |
inline ref.read(accordAuthProvider.select(...)) still present; plus the now-dead accord_auth imports on lines 4–5 were not removed |
lib/features/user/views/accord_direct_messages.dart |
openAccordDirectMessage() line 78 |
same inline pattern |
lib/features/spaces/views/accord_home.dart |
_ackLast() line 353 |
same inline pattern (the client_access.dart import was added for the part-files, but this call site in the host file wasn't migrated) |
Fix applied: replaced all three with ref.accordClient; removed the two dead auth imports from channel_management.dart.
🟡 MEDIUM — No tests for the two new shared utilities
SectionHeader and showConfirmDialog were introduced without any test coverage. Given that they now serve as the single source of truth for a rendering pattern and a UX guard used across ~10 screens, regressions here would be silent and wide.
Fix applied: added test/shared/section_header_test.dart and test/shared/confirm_dialog_test.dart covering:
SectionHeader: uppercase rendering, correct padding, key forwardingshowConfirmDialog: title/message rendering, confirm→true, cancel→false, custom labels, danger mode setsbackgroundColor, non-danger mode leaves it null
🟢 LOW — No issues found in the rest of the PR
- All 3 new shared files (
section_header.dart,confirm_dialog.dart,client_access.dart) are clean, well-structured, and consistent with project conventions. - The
SectionHeaderextracted from 4 identical private widgets is pixel-identical to each original (same padding, same style, same uppercase transform). - The
showConfirmDialoghelper preserves all prior variants (plain confirm, danger-red confirm) and normalises the two outliers that previously usedTextButtoninstead ofFilledButton— the PR description calls this out as a deliberate visual normalisation, which is correct behaviour. - The
AccordClientWidgetRef/AccordClientRefextension pair correctly centralises theaccordAuthProvider.select(...)lookup. Files that still import auth directly do so for non-client selectors (e.g.s.session.userId,s.session.server.cdnUrl) that are legitimately out of scope for this PR. - No file size issues (all touched files remain well under 500 lines).
- No Discord endpoints, Firebase code, or licence issues introduced.
Generated by Claude Code
Merge master (which substantially rewrote thread_view.dart to use accordAuthProvider) into the branch. The 3-way merge kept master's body but my removal of thread_view's auth imports, breaking analysis. Re-add the auth imports, convert master's new raw _client getter to ref.accordClient, and fold accord_space_settings' identical _SectionHeader into the shared widget.
… claude/upbeat-knuth-rz2tn5
The import-insertion guard matched a doc comment mentioning the helper path and skipped adding the actual import, leaving SectionHeader undefined. Add the real import.
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.
What
A WET-code reduction sweep across
lib/. Three independent, mechanical consolidations, each in its own commit (net −171 lines, 36 files):d0fa14e) — theaccordAuthProvider.select((s) => s is AccordAuthLoggedIn ? s.client : null)expression was copy-pasted into 28ConsumerState/Refgetters. Extracted once asref.accordClient(WidgetRef/Refextensions inshared/utils/client_access.dart); each_clientgetter now delegates, and 22 now-unused auth imports were dropped.027c3be) — four identical private_SectionHeader/_Headerwidgets (settings, developer, voice, privacy) replaced by oneshared/components/section_header.dart.cf405c2) — eight duplicated Cancel/confirmAlertDialogs (three admin_confirmmethods + five inline) replaced by oneshowConfirmDialog()helper with optionaldangerstyling.Notes for review
TextButton(member popout, channel delete) or MaterialColors.red/colorScheme.error(emoji delete, privacy leave) now render the standard themedFilledButton/red used everywhere else — deliberate, for consistency.flutter analyze/test run.Not included
A fourth candidate — extracting the repeated async
_busy/_error+setStatepattern (~74 sites) into a shared mixin — was intentionally left out: it alters control flow across many heterogeneous State classes and warrants compiler-verified, more careful treatment.https://claude.ai/code/session_014TxF8FfETRJE6bHUi9q1EV
Generated by Claude Code