Skip to content

test(pocket): stage push-disabled case in NeedsHomeScreenInstall story - #360

Open
dormouse-bot wants to merge 1 commit into
mainfrom
fix/hosts-story-needs-install-disabled
Open

test(pocket): stage push-disabled case in NeedsHomeScreenInstall story#360
dormouse-bot wants to merge 1 commit into
mainfrom
fix/hosts-story-needs-install-disabled

Conversation

@dormouse-bot

Copy link
Copy Markdown
Collaborator

Nightly survey finding.

The NeedsHomeScreenInstall story in HostsView.stories.tsx set only args: { pushState: 'needs-install' } — byte-identical to the PushNeedsInstall story a few lines above it — so the two produced the same render and two identical Chromatic snapshots.

Worse, its docstring describes a case it never staged: "A definitively push-disabled Server hides the notice so it cannot disagree with the push row beneath it." That behavior is gated in HostsView on both conditions:

{pushConfigStatus !== 'disabled' && pushState === 'needs-install' ? (
  <InstallNotice />
) : null}

With the default pushConfigStatus: 'ready' the guard was truthy, so the story rendered the install notice — the opposite of what the docstring claims. Adding pushConfigStatus: 'disabled' makes the story actually stage the documented state: the InstallNotice is suppressed and each host row instead shows "This server has push notifications disabled." (App.tsx, the pushConfigStatus === 'disabled' branch of pushCopy). This distinguishes it from both PushNeedsInstall (notice shown) and PushUnconfigured (pushState left at ready).

No unit test is added: the story itself is the test artifact, and its Chromatic snapshot now captures the notice-hidden state it was always meant to cover. This is a visually-changing PR, so the UI Tests diff needs a maintainer's review in Chromatic.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 44da454
Status: ✅  Deploy successful!
Preview URL: https://ce523876.mouseterm.pages.dev
Branch Preview URL: https://fix-hosts-story-needs-instal.mouseterm.pages.dev

View logs

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant