Skip to content

Replace NPendingRequests with a placeable counter #746

Description

@nogringo

Replace NPendingRequests with a placeable counter

Labels: enhancement

Problem

packages/ndk_flutter/lib/widgets/pending_requests/n_pending_requests.dart does too
much and too little at the same time.

Too much: it ships an expand/collapse panel with a request list, per-request method
and kind chips, relative timestamps and a cancel button. That is a worse version of a
dedicated screen, and packages/sample-app/lib/pending_requests_page.dart already
implements the good version in 694 lines.

Too little: it only ever shows one account
(getLoggedAccount().signer.pendingRequestsStream, lines 85 to 100).

It is also a bad citizen on mobile. Being an AnimatedPositioned inside a required
Stack, it floats over the content and collides with FABs, bottom navigation,
snackbars and the keyboard.

Bugs it carries today

Most of these disappear with the widget rather than needing individual fixes:

  • ListView.builder with shrinkWrap: true (line 299) over a potentially large list.
  • _formatTime (line 426) is never refreshed, so "3s ago" stays frozen until the
    stream re-emits.
  • _cancelRequest (line 135) routes to the logged account's signer. For a request
    belonging to another account the id is not in that signer's map, so
    cancelRequest returns false and the button silently does nothing.
  • _signerName (line 111) identifies the signer with
    runtimeType.toString().contains('Nip46').
  • The panel appears the instant a request is registered, so a remote signer in
    auto-approve mode makes the whole thing flash in and out on every signature.

Suggested replacement

NPendingRequestsIndicator: a count and an onTap callback, nothing else. The host
app decides what tapping does, typically pushing its own dedicated screen.

  • Not self-positioning. A plain widget the developer places where they want, for
    example in AppBar.actions. This removes the Stack requirement and the mobile
    collisions.
  • A single optional builder(context, count) instead of the current
    collapsedBuilder and expandedBuilder.
  • Scope defaults to all accounts, with an optional pubkey parameter to restrict to
    one. The default should not lie about how many requests are pending.
  • Keep a short delay before showing (roughly 600ms to 1s) so auto-approved requests
    never render, and a minimum visible duration so it does not flash on the way out.
    This delay belongs here, not in the stream.

Docs

Cancelling a request stays available through Account.cancelRequest
(packages/ndk/lib/domain_layer/entities/account.dart:28). The docs must point at it
explicitly, otherwise dropping the panel reads as a regression.

Breaking change

NPendingRequests, collapsedBuilder and expandedBuilder are public API of
ndk_flutter.

To decide: remove outright, or deprecate for one release cycle with the new indicator
alongside. Deprecating costs a follow-up cleanup issue but avoids breaking consumers
in the same version that changes emission timing.

Depends on

The aggregated pending-requests stream issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions