Skip to content

Fix Linear skill-install modal CTA hierarchy - #9643

Merged
brennanb2025 merged 5 commits into
mainfrom
brennanb2025/skill-install-decline-default
Jul 21, 2026
Merged

brennanb2025 merged 5 commits into
mainfrom
brennanb2025/skill-install-decline-default

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

What

Fixes the action hierarchy of the Linear agent-skill install modal. Neil + Brennan flagged that the dismiss button read as the primary CTA and got reflexively clicked.

Linear skill-install modal — final

Changes

  • Install CLI & Skill → filled primary (default). Added an opt-in installVariant prop to AgentSkillSetupPanel (defaults to outline, so settings surfaces are untouched); the Linear modal passes default.
  • Removed the footer dismiss buttons. The footer is gone entirely, so Install is the only labeled action.
  • Permanent opt-out → a quiet EyeOff icon next to the ×, matching the house pattern for hiding a setup/teaching surface (SetupGuideModal, StatusBarUsageEmptyCta, SetupGuideSidebarEntry). The two "make it go away" controls now sit together — × = dismiss for the session, ⊘ EyeOff = don't show again — and neither is a tempting text button.
  • Initial focus lands on Install (the EyeOff renders last in the branch) so Enter-on-open can't permanently dismiss. Wrapped in a local TooltipProvider so it works in isolated mounts/tests.

Testing

  • LinearAgentSkillSetupPrompt.test.tsx: 27/27 pass (× → session snooze; EyeOff → permanent dismiss, located by aria-label).
  • Electron QA: mounted the real dialog in the running app — verified Install=default, EyeOff present (aria "Don't show again"), no footer, and initial focus = Install (not the hide button). Screenshot above.
  • tsc (web) + oxlint clean; all 5 locale JSONs validate.

Note: the modal also auto-opens on the first activation of a Linear-linked worktree with the skill missing (not only via the reminder toast's "Set up"). For that path the × (session) and the EyeOff (permanent) cover dismissal.

Remove the redundant 'Not now' button from the Linear agent-skill
install modal and make Install the filled primary action, matching the
other setup surfaces (filled primary + muted dismiss).

'Not now' was doing exactly what the dialog's × already does (session
snooze via onOpenChange), but styled as an outline button in the
footer-right CTA slot it read as the primary action over Install —
which itself was only an outline button buried in the panel body. Users
reflexively clicked the dismiss.

- Drop 'Not now' + its unused onSnoozeForSession prop / i18n key (all locales)
- Add opt-in installVariant to AgentSkillSetupPanel (defaults 'outline';
  settings surfaces unchanged) and pass 'default' from the Linear modal
- Update tests to drive the session snooze via the dialog ×
@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The installer setup panel now accepts an optional button variant, defaults to outline, and applies it to its install or update CTA. The Linear setup dialog passes the default variant, removes its session-only “Not now” action, and retains permanent dismissal. Modal close handling now performs session snoozing, with corresponding test and localization updates.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning It includes a summary and testing, but it omits required AI Review Report, Security Audit, and explicit Notes sections from the template. Add the missing template sections: AI Review Report, Security Audit, and Notes, and describe the cross-platform review and any follow-up risks.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed It accurately summarizes the main change: reworking the Linear skill-install modal's CTA hierarchy.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brennanb2025

brennanb2025 commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor Author

⚠️ Superseded — see the revised design comment and the PR description.
This first pass removed "Not now". We reverted that: because the modal auto-opens unsolicited, the reflexive dismiss must stay non-destructive, so "Not now" is kept (quiet ghost = session snooze) and "Don't show again" is demoted to a muted link. The screenshots below are outdated.


Electron QA — visible modal render

Launched the dev build on this branch (brennanb2025/skill-install-decline-default, CDP-attached, verified instance identity) and rendered the real LinearAgentSkillSetupDialog to confirm the shipped CTA hierarchy on screen.

Dialog close-up:

Linear skill-install modal — new CTA hierarchy

Full app window:

Full window with modal open

What the render confirms

  • "Install CLI & Skill" is now the filled primary (data-variant="default") — it visually reads as the action.
  • "Not now" is gone. Footer holds only the quiet ghost "Don't show again"; the dialog × still handles the session snooze.
  • Re-check stays a muted ghost button.

DOM assertion from the running app:

{"buttons":[{"text":"Install CLI & Skill","variant":"default"},{"text":"Re-check","variant":"ghost"},{"text":"Don't show again","variant":"ghost"},{"text":"Close","variant":null}],"hasNotNow":false,"hasDontShow":true,"hasInstall":true}

Honesty note on method

This dev profile had no Linear-linked worktree, and the modal is gated on isActive && worktree.linkedLinearIssue + missing skill, so I mounted the real, unmodified LinearAgentSkillSetupDialog component (same React instance as the app, real CSS/theme) rather than driving the full product flow to reach it. The styling/hierarchy shown is the genuine component output; the trigger path (link a Linear ticket → skill missing) was not exercised here.

…ted link

This modal auto-opens unsolicited (first activation of a Linear-linked
worktree with the skill missing), so the reflexive dismiss must be
non-destructive. Removing 'Not now' left the permanent 'Don't show again'
as the only labeled footer button — the reflexive target — which would
nuke a useful prompt forever.

Restore 'Not now' (session snooze) as a quiet ghost — the easy, safe
dismiss, matching Neil's original ask ('not now is fine, just not the
CTA'). Demote 'Don't show again' to a muted text link so permanent
suppression takes intent. Install stays the filled primary.

Restores onSnoozeForSession wiring + notNow i18n key (all locales);
tests exercise the Not now snooze path again.
@brennanb2025 brennanb2025 changed the title Fix Linear skill-install modal CTA hierarchy (remove redundant 'Not now') Fix Linear skill-install modal CTA hierarchy Jul 21, 2026
@brennanb2025

brennanb2025 commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor Author

⚠️ Superseded — see the final comment and the PR description. "Not now" was ultimately removed; the × handles session dismiss. Screenshot below is outdated.


Design update — safe reflexive dismiss (revised from the first pass)

Since this modal auto-opens unsolicited, the earlier "just remove Not now" pass had a side effect: it left permanent "Don't show again" as the only labeled footer button — i.e. the reflexive dismiss became the destructive one, nuking a useful prompt forever.

Revised layout (verified live in the running app):

Final modal footer

  • Install CLI & Skill — filled primary (data-variant="default")
  • Not now — quiet ghost, footer-right = the easy/safe reflexive dismiss (session snooze)
  • Don't show again — muted link, footer-left = permanent, takes intent
  • × — still snoozes for the session

Live DOM assertion:

{"buttons":[{"text":"Install CLI & Skill","variant":"default"},{"text":"Re-check","variant":"ghost"},{"text":"Don't show again","variant":"link"},{"text":"Not now","variant":"ghost"},{"text":"Close","variant":null}]}
Full app window

Full window

Method note: this dev profile had no Linear-linked worktree, so I mounted the real, unmodified LinearAgentSkillSetupDialog (same React instance + CSS as the app) rather than driving the full link-ticket flow. Styling/hierarchy shown is genuine component output.

…ismiss

Per product call: the modal is typically reached by clicking 'Set up' on
the reminder toast (an opt-in), and the dialog's × already dismisses for
the session. Drop the redundant 'Not now' button. Footer keeps only the
quiet muted 'Don't show again' (permanent opt-out); Install stays the
filled primary.

Removes onSnoozeForSession wiring + notNow i18n key (all locales); tests
exercise the × (Close) session-snooze path.
@brennanb2025

brennanb2025 commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor Author

⚠️ Superseded — see the latest comment and the PR description. The permanent opt-out moved from a footer link to an EyeOff icon next to the ×. Screenshot below is outdated.


Final: "Not now" removed

Landed on removing "Not now" entirely — the modal is normally opened by clicking "Set up" on the reminder toast (opt-in), and the dialog × already covers session dismiss, so it was redundant.

Final modal

Live DOM from the running app:

{"buttons":[{"text":"Install CLI & Skill","variant":"default"},{"text":"Re-check","variant":"ghost"},{"text":"Don't show again","variant":"link"},{"text":"Close","variant":null}],"hasNotNow":false}
  • Install CLI & Skill — filled primary
  • Don't show again — quiet muted link (permanent opt-out)
  • × — session dismiss

Supersedes the earlier "keep Not now" comment.

…GuideModal)

Replaces the footer 'Don't show again' link with a quiet EyeOff icon
button beside the dialog ×, matching the house pattern for hiding a
setup/teaching surface (SetupGuideModal, StatusBarUsageEmptyCta,
SetupGuideSidebarEntry). The footer is gone entirely, so Install is the
only labeled action; the two 'make it go away' controls (× = for now,
EyeOff = never) sit together and neither is a tempting text button.

- Rendered last in the branch so initial dialog focus lands on Install,
  not the hide button (Enter-on-open must not permanently dismiss).
- Wrapped in a local TooltipProvider so it works outside the app's global
  provider (isolated tests/mounts).
- Reuses the existing dontShowAgain i18n key for aria-label + tooltip.
- Tests locate the control by aria-label now that it has no text.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Update — permanent opt-out is now an EyeOff icon (no footer)

Replaced the muted "Don't show again" footer link with a quiet EyeOff icon button next to the ×, matching the house pattern for hiding a setup surface (SetupGuideModal et al.). Footer is gone — Install is the only labeled action.

Final modal

  • **× ** = dismiss for the session · ⊘ EyeOff = don't show again (tooltip + aria-label)
  • Initial focus lands on Install (EyeOff renders last) so Enter-on-open can't nuke it.

Live-verified in the app: activeElement on open = "Install CLI & Skill". Supersedes the earlier layouts on this PR.

The icon-xs button is 24px tall vs the bare 16px × close, so its centered
icon sat 2px low. top-3 lines the two icon centers up (measured pixel-exact
in the running app).
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Nit fix — EyeOff now pixel-aligned with the ×

The hide icon sat 2px low (its icon-xs button is 24px tall vs the bare 16px × close). Nudged top-3.5→top-3; both icon centers now measure Y=433 in the running app.

Corner alignment

@brennanb2025
brennanb2025 merged commit 827cd49 into main Jul 21, 2026
1 check passed
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
* Fix Linear skill-install modal CTA hierarchy

Remove the redundant 'Not now' button from the Linear agent-skill
install modal and make Install the filled primary action, matching the
other setup surfaces (filled primary + muted dismiss).

'Not now' was doing exactly what the dialog's × already does (session
snooze via onOpenChange), but styled as an outline button in the
footer-right CTA slot it read as the primary action over Install —
which itself was only an outline button buried in the panel body. Users
reflexively clicked the dismiss.

- Drop 'Not now' + its unused onSnoozeForSession prop / i18n key (all locales)
- Add opt-in installVariant to AgentSkillSetupPanel (defaults 'outline';
  settings surfaces unchanged) and pass 'default' from the Linear modal
- Update tests to drive the session snooze via the dialog ×

* Keep 'Not now' as the safe dismiss; demote 'Don't show again' to a muted link

This modal auto-opens unsolicited (first activation of a Linear-linked
worktree with the skill missing), so the reflexive dismiss must be
non-destructive. Removing 'Not now' left the permanent 'Don't show again'
as the only labeled footer button — the reflexive target — which would
nuke a useful prompt forever.

Restore 'Not now' (session snooze) as a quiet ghost — the easy, safe
dismiss, matching Neil's original ask ('not now is fine, just not the
CTA'). Demote 'Don't show again' to a muted text link so permanent
suppression takes intent. Install stays the filled primary.

Restores onSnoozeForSession wiring + notNow i18n key (all locales);
tests exercise the Not now snooze path again.

* Remove 'Not now' from Linear skill-install modal; × handles session dismiss

Per product call: the modal is typically reached by clicking 'Set up' on
the reminder toast (an opt-in), and the dialog's × already dismisses for
the session. Drop the redundant 'Not now' button. Footer keeps only the
quiet muted 'Don't show again' (permanent opt-out); Install stays the
filled primary.

Removes onSnoozeForSession wiring + notNow i18n key (all locales); tests
exercise the × (Close) session-snooze path.

* Move permanent opt-out to an EyeOff icon next to the × (matches SetupGuideModal)

Replaces the footer 'Don't show again' link with a quiet EyeOff icon
button beside the dialog ×, matching the house pattern for hiding a
setup/teaching surface (SetupGuideModal, StatusBarUsageEmptyCta,
SetupGuideSidebarEntry). The footer is gone entirely, so Install is the
only labeled action; the two 'make it go away' controls (× = for now,
EyeOff = never) sit together and neither is a tempting text button.

- Rendered last in the branch so initial dialog focus lands on Install,
  not the hide button (Enter-on-open must not permanently dismiss).
- Wrapped in a local TooltipProvider so it works outside the app's global
  provider (isolated tests/mounts).
- Reuses the existing dontShowAgain i18n key for aria-label + tooltip.
- Tests locate the control by aria-label now that it has no text.

* Align EyeOff hide icon with the × (top-3 not top-3.5)

The icon-xs button is 24px tall vs the bare 16px × close, so its centered
icon sat 2px low. top-3 lines the two icon centers up (measured pixel-exact
in the running app).
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