Skip to content

Fix modal button pointer and replication action click#1789

Merged
riderx merged 8 commits into
mainfrom
riderx/fix-repl-pointer
Mar 12, 2026
Merged

Fix modal button pointer and replication action click#1789
riderx merged 8 commits into
mainfrom
riderx/fix-repl-pointer

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented Mar 12, 2026

Summary (AI generated)

  • Fixed click accessibility for the replication toast action button by adding an explicit pointer cursor to its Vue-rendered button.
  • Kept the existing DialogV2 flow for deployment/update dialogs, including improved disabled handling and secure external-link handling.

Motivation (AI generated)

The replication bottom-toast action had a “no pointer” affordance and could appear unclickable, reducing discoverability during active replication updates.

Business Impact (AI generated)

This reduces friction for users trying to navigate from replication notifications, improving responsiveness in operational workflows.

Test Plan (AI generated)

  • Run bun lint
  • Manually verify the replication toast action is clickable and shows a pointer cursor on hover

Summary by CodeRabbit

  • Improvements
    • Enhanced dialog button interactions, including improved handling of buttons that function as links and better disabled-state behavior.
    • Added visual cursor feedback when hovering over action buttons in notification toasts.
    • Refined dialog navigation logic for improved user experience when interacting with dialog buttons.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 12, 2026

📝 Walkthrough

Walkthrough

Dialog button rendering logic in DialogV2.vue is refactored from dynamic component switching to explicit rendering paths for button and anchor elements with unified click handling. The store's dialog close flow is modified to handle href navigation via a new openButtonHref utility. An action button styling update adds cursor-pointer to the toast service.

Changes

Cohort / File(s) Summary
Dialog Button Refactoring
src/components/DialogV2.vue, src/stores/dialogv2.ts
Explicit button/anchor rendering paths replace dynamic components; unified handleButtonClick handler processes clicks with disabled-state and link-modified-click checks. New openButtonHref utility manages navigation with target/rel support and fallback to window.location.assign. Dialog close flow modified to invoke href navigation and ensure dialog is hidden when dismissed without button action.
Toast Styling
src/services/updateReplicationToast.ts
Added cursor-pointer CSS class to action button when actionLabel and onAction are present, providing visual feedback on hover.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Buttons now render with purpose clear,
Explicit paths, no guessing here!
Click handlers unified, href on the way,
Dialog flows gracefully, hip-hop hooray!
A cursor pointer winks at you,
Toast to the changes—they're shiny and new!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description covers summary and test plan but is missing required sections (manual test steps and checklist items) from the template. Complete the description by adding explicit manual test reproduction steps and checking the code style/documentation checklist items.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title directly matches the main changes: fixing modal button pointer styling and replication action click behavior across three files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch riderx/fix-repl-pointer
📝 Coding Plan
  • Generate coding plan for human review comments

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/components/DialogV2.vue (1)

118-121: Redundant and conflicting cursor classes when disabled.

When button.disabled is true:

  • cursor-not-allowed is applied twice (lines 119 and 120)
  • pointer-events-none makes the cursor style invisible anyway since the element won't receive pointer events

Consider simplifying:

♻️ Suggested simplification
                 :class="{
                   'd-btn d-btn-primary': button.role === 'primary',
                   'd-btn d-btn-secondary': button.role === 'secondary',
                   'd-btn d-btn-warning': button.role === 'danger',
                   'd-btn d-btn-outline': button.role === 'cancel',
                   'd-btn': !button.role,
-                  '!cursor-pointer': !button.disabled,
-                  'cursor-not-allowed': button.disabled,
-                  'opacity-70 cursor-not-allowed pointer-events-none': button.disabled,
+                  'cursor-pointer': !button.disabled,
+                  'opacity-70 pointer-events-none': button.disabled,
                 }"

The same applies to the anchor element at lines 139-141.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DialogV2.vue` around lines 118 - 121, The class binding on the
button and the anchor in DialogV2.vue applies redundant/conflicting classes when
button.disabled is true (the object containing '!cursor-pointer',
'cursor-not-allowed', 'opacity-70 cursor-not-allowed pointer-events-none');
remove the duplicate 'cursor-not-allowed' and drop 'pointer-events-none' (since
it prevents cursor behavior) and instead conditionally apply a single disabled
set like 'cursor-not-allowed opacity-70' when button.disabled is true and
'!cursor-pointer' when false; update both the button and the anchor class
binding objects to use these simplified keys so the disabled styling is
unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/DialogV2.vue`:
- Around line 118-121: The class binding on the button and the anchor in
DialogV2.vue applies redundant/conflicting classes when button.disabled is true
(the object containing '!cursor-pointer', 'cursor-not-allowed', 'opacity-70
cursor-not-allowed pointer-events-none'); remove the duplicate
'cursor-not-allowed' and drop 'pointer-events-none' (since it prevents cursor
behavior) and instead conditionally apply a single disabled set like
'cursor-not-allowed opacity-70' when button.disabled is true and
'!cursor-pointer' when false; update both the button and the anchor class
binding objects to use these simplified keys so the disabled styling is
unambiguous.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6ed5cd93-dbc9-4e99-9678-8e21e2bc633d

📥 Commits

Reviewing files that changed from the base of the PR and between a26c98b and 01be4b6.

📒 Files selected for processing (3)
  • src/components/DialogV2.vue
  • src/services/updateReplicationToast.ts
  • src/stores/dialogv2.ts

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 01be4b647b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/stores/dialogv2.ts
Comment on lines +86 to +87
// Modal dismissed without a button action (overlay, escape, close icon)
showDialog.value = false
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore cancel state on overlay/Escape dismissal

When the dialog is closed without a button (closeDialog() called with no argument), this path now only hides the modal and leaves dialogCanceled as false. That flips the meaning of onDialogDismiss() for backdrop/Escape/X closes, so those dismissals are treated as confirmations in existing callers that use !await dialogStore.onDialogDismiss() (for example the bundle delete confirmation flow in src/components/tables/BundleTable.vue), which can trigger destructive actions after a user dismisses the modal.

Useful? React with 👍 / 👎.

@sonarqubecloud
Copy link
Copy Markdown

@riderx riderx merged commit c485e5a into main Mar 12, 2026
14 of 15 checks passed
@riderx riderx deleted the riderx/fix-repl-pointer branch March 12, 2026 20:08
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