Skip to content

Add titles to icon buttons #445

Merged
Wirasm merged 2 commits into
coleam00:mainfrom
djankies:feature/icon-button-tooltips
Aug 25, 2025
Merged

Add titles to icon buttons #445
Wirasm merged 2 commits into
coleam00:mainfrom
djankies:feature/icon-button-tooltips

Conversation

@djankies

@djankies djankies commented Aug 22, 2025

Copy link
Copy Markdown

Adds tooltips to icon buttons

Changes Made

  • Added title attributes to icon buttons in DocumentCard component
  • Added title attributes to icon buttons (delete, edit, view details) in DraggableTaskCard component
  • Added title attributes to icon buttons (delete, complete, edit) in TaskTableView component

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Affected Services

  • Frontend (React UI)
  • Server (FastAPI backend)
  • MCP Server (Model Context Protocol)
  • Agents (PydanticAI service)
  • Database (migrations/schema)
  • Docker/Infrastructure
  • Documentation site

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Manually tested affected user flows
  • Docker builds succeed for all services

Test Evidence

cd archon-ui-main && npm test -- --run
# Results: 74 tests passed, 3 failed (unrelated to UI changes - API config issues)

Checklist

  • My code follows the service architecture patterns
  • If using an AI coding assistant, I used the CLAUDE.md rules
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally
  • My changes generate no new warnings
  • I have updated relevant documentation
  • I have verified no regressions in existing features

Breaking Changes

None - this is a non-breaking enhancement that only adds tooltips to existing buttons.

Additional Notes

  • The 3 failing tests are unrelated to these UI changes and appear to be pre-existing issues with API configuration and MCP client service
  • Tooltips improve accessibility by providing clear descriptions of button actions
  • No new dependencies were added

Summary by CodeRabbit

  • Style
    • Added explicit button types and presentational/accessibility attributes to Project Tasks actions: titles and aria-labels (e.g., Delete, Edit, View details, Flip, Copy ID) and icons marked aria-hidden.
  • Chores
    • UI-only accessibility improvements; no behavioral, data-flow, or public API changes.

@coderabbitai

coderabbitai Bot commented Aug 22, 2025

Copy link
Copy Markdown

Walkthrough

Adds explicit type="button", title attributes, and aria-* adjustments to action buttons in DocumentCard, DraggableTaskCard, and TaskTableView (including making icon elements aria-hidden="true"). No changes to event handlers, control flow, exports, or business logic.

Changes

Cohort / File(s) Summary of Changes
Document card accessibility
archon-ui-main/src/components/project-tasks/DocumentCard.tsx
Added type="button" to the Copy Document ID and Delete buttons; added aria-label="Copy Document ID to clipboard" to the copy button; added title="Delete document" to the delete button; icons set to aria-hidden="true". No behavioral changes.
Draggable task card accessibility
archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx
Added type="button" to all front/back action buttons (Delete, Edit, View details, Copy Task ID, Flip back); added title and aria-label attributes for Delete/Edit/View/Flip; Copy Task ID gets aria-label="Copy Task ID to clipboard"; icons set to aria-hidden="true". Minor whitespace trim. No logic changes.
Task table row accessibility
archon-ui-main/src/components/project-tasks/TaskTableView.tsx
In DraggableTaskRow action controls added type="button" for buttons; added title and aria-label for Delete, Mark complete, Edit, and Copy Task ID; icons set to aria-hidden="true". No behavioral changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I press a tiny, mindful paw,
A label wakes with gentle law,
Icons hush while helpers speak,
No tangled flows, no logic tweak.
Hop! — a clearer, kinder click.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
archon-ui-main/src/components/project-tasks/TaskTableView.tsx (1)

367-367: Make action buttons visible on keyboard focus

The action group is opacity-0 unless hovered, which hides controls from keyboard users. Reveal via focus-within.

-        <div className="flex justify-center gap-2 opacity-0 group-hover:opacity-100 transition-opacity">
+        <div className="flex justify-center gap-2 opacity-0 group-hover:opacity-100 focus-within:opacity-100 transition-opacity">
archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx (1)

147-156: Icon-only buttons need aria-labels; add aria-hidden on SVGs and type="button"

Title alone won’t give a robust accessible name. Add aria-label to each button, mark the icons aria-hidden, and set type="button".

-                <button 
+                <button 
+                  type="button"
                   onClick={(e) => {
                     e.stopPropagation();
                     onDelete(task);
                   }} 
                   className="w-5 h-5 rounded-full flex items-center justify-center bg-red-100/80 dark:bg-red-500/20 text-red-600 dark:text-red-400 hover:bg-red-200 dark:hover:bg-red-500/30 hover:shadow-[0_0_10px_rgba(239,68,68,0.3)] transition-all duration-300"
                   title="Delete task"
+                  aria-label="Delete task"
                 >
-                  <Trash2 className="w-3 h-3" />
+                  <Trash2 className="w-3 h-3" aria-hidden="true" />
                 </button>
-                <button 
+                <button 
+                  type="button"
                   onClick={(e) => {
                     e.stopPropagation();
                     onView();
                   }} 
                   className="w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300"
                   title="Edit task"
+                  aria-label="Edit task"
                 >
-                  <Edit className="w-3 h-3" />
+                  <Edit className="w-3 h-3" aria-hidden="true" />
                 </button>
-                <button 
+                <button 
+                  type="button"
                   onClick={toggleFlip} 
                   className="w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300"
                   title="View task details"
+                  aria-label="View task details"
                 >
-                  <RefreshCw className="w-3 h-3" />
+                  <RefreshCw className="w-3 h-3" aria-hidden="true" />
                 </button>
...
-              <button 
+              <button 
+                type="button"
                 onClick={toggleFlip} 
                 className="ml-auto w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300"
                 title="Flip back to front"
+                aria-label="Flip back to front"
               >
-                <RefreshCw className="w-3 h-3" />
+                <RefreshCw className="w-3 h-3" aria-hidden="true" />
               </button>

Also applies to: 157-166, 168-173, 227-233

🧹 Nitpick comments (3)
archon-ui-main/src/components/project-tasks/TaskTableView.tsx (1)

383-386: Confirm label vs action: “Edit task” triggers onTaskView

If onTaskView opens a read-only details view, the tooltip/aria-label should read “View task details” for consistency with the board card. If it opens an editor, keep “Edit task” and align the card copy.

archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx (2)

162-164: Align label with action (“Edit task” calls onView)

Same note as the table view: confirm whether onView opens an editor or a read-only details view, and align the tooltip copy accordingly across components.


244-244: Remove trailing whitespace

Cosmetic, but it avoids lint noise and spurious diffs.

-      </div> 
+      </div>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 86dd1b0 and 044b04f.

📒 Files selected for processing (3)
  • archon-ui-main/src/components/project-tasks/DocumentCard.tsx (1 hunks)
  • archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx (4 hunks)
  • archon-ui-main/src/components/project-tasks/TaskTableView.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}: Remove dead code immediately; do not keep legacy/unused functions
Avoid comments that reference change history (e.g., LEGACY, CHANGED, REMOVED); keep comments focused on current functionality

Files:

  • archon-ui-main/src/components/project-tasks/TaskTableView.tsx
  • archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx
  • archon-ui-main/src/components/project-tasks/DocumentCard.tsx
archon-ui-main/src/components/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components in archon-ui-main/src/components/

Files:

  • archon-ui-main/src/components/project-tasks/TaskTableView.tsx
  • archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx
  • archon-ui-main/src/components/project-tasks/DocumentCard.tsx
🔇 Additional comments (1)
archon-ui-main/src/components/project-tasks/DocumentCard.tsx (1)

121-121: Good addition; pairs well with existing aria-label

Adding title="Delete document" complements the aria-label and provides a native tooltip without changing behavior. LGTM.

Comment thread archon-ui-main/src/components/project-tasks/TaskTableView.tsx
@Wirasm

Wirasm commented Aug 23, 2025

Copy link
Copy Markdown
Collaborator

Thank you @djankies this is a great contribution. feel free to make a follow up pr or incorporate the code rabbit suggestions in here as well!

Add type, aria-label, and aria-hidden attributes to action and icon buttons across task and document components to improve accessibility and assistive technology support.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (7)
archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx (7)

148-158: Solid accessibility improvements; add a visible keyboard focus style.

Nice additions: type="button", aria-label, and aria-hidden on the icon. Add a focus-visible ring so keyboard users can see focus without relying on hover.

Apply:

-                  className="w-5 h-5 rounded-full flex items-center justify-center bg-red-100/80 dark:bg-red-500/20 text-red-600 dark:text-red-400 hover:bg-red-200 dark:hover:bg-red-500/30 hover:shadow-[0_0_10px_rgba(239,68,68,0.3)] transition-all duration-300"
+                  className="w-5 h-5 rounded-full flex items-center justify-center bg-red-100/80 dark:bg-red-500/20 text-red-600 dark:text-red-400 hover:bg-red-200 dark:hover:bg-red-500/30 hover:shadow-[0_0_10px_rgba(239,68,68,0.3)] transition-all duration-300 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-red-400 dark:focus-visible:ring-red-300"

160-170: Verify action semantics: “Edit task” calls onView().

If onView opens an edit modal/view, all good. If it merely opens a read-only view, the label “Edit task” is misleading, impacting a11y. Consider renaming the handler to onEdit (and wiring appropriately) or adjust the label.

Also add a keyboard focus style:

-                  className="w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300"
+                  className="w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-400 dark:focus-visible:ring-cyan-300"

172-179: Good “View task details” labeling; add focus-visible styling.

The tooltip + aria-label are consistent. Add a visible focus ring for keyboard users.

-                  className="w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300"
+                  className="w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-400 dark:focus-visible:ring-cyan-300"

200-218: Avoid innerHTML mutation; handle clipboard failures; add screen-reader feedback.

Direct DOM mutation via innerHTML is a React anti-pattern and risks brittle UI. Clipboard writes can fail (permissions, insecure context). Prefer state-driven feedback and an aria-live status for SR users.

Diff in-place:

-                onClick={(e) => {
+                onClick={async (e) => {
                   e.stopPropagation();
-                  navigator.clipboard.writeText(task.id);
-                  // Optional: Add a small toast or visual feedback here
-                  const button = e.currentTarget;
-                  const originalHTML = button.innerHTML;
-                  button.innerHTML = '<span class="text-green-500">Copied!</span>';
-                  setTimeout(() => {
-                    button.innerHTML = originalHTML;
-                  }, 2000);
+                  try {
+                    await navigator.clipboard.writeText(task.id);
+                    setCopied(true);
+                    setTimeout(() => setCopied(false), 2000);
+                  } catch (err) {
+                    console.error('Failed to copy Task ID:', err);
+                  }
                 }}
                 className="flex items-center gap-1 text-xs text-gray-500 hover:text-gray-700 dark:text-gray-400 dark:hover:text-gray-200 transition-colors"
                 title="Copy Task ID to clipboard"
                 aria-label="Copy Task ID to clipboard"
               >
-                <Clipboard className="w-3 h-3" aria-hidden="true" />
-                <span>Task ID</span>
+                <Clipboard className="w-3 h-3" aria-hidden="true" />
+                <span>Task ID</span>
+                <span className="sr-only" role="status" aria-live="polite">{copied ? 'Copied to clipboard' : ''}</span>
               </button>

Add the supporting state near Line 61:

const [copied, setCopied] = useState(false);

Optionally, add a small visual indicator (e.g., swap label to “Copied!”) conditionally instead of mutating innerHTML. I can provide a follow-up patch if you want that UX tweak as well.


236-243: Back-side flip button: add visible focus style for keyboard users.

Consistent with the front-side actions, add a focus-visible ring.

-                className="ml-auto w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300"
+                className="ml-auto w-5 h-5 rounded-full flex items-center justify-center bg-cyan-100/80 dark:bg-cyan-500/20 text-cyan-600 dark:text-cyan-400 hover:bg-cyan-200 dark:hover:bg-cyan-500/30 hover:shadow-[0_0_10px_rgba(34,211,238,0.3)] transition-all duration-300 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-cyan-400 dark:focus-visible:ring-cyan-300"

148-158: Consider a reusable IconButton with an accessible tooltip.

You’re repeating similar button markup (classes, sizing, aria-label/title/icon). A shared IconButton that:

  • Accepts icon, label, tone (red/cyan/etc.), onClick, and tooltip props
  • Applies aria-hidden to icons by default, ensures type="button"
  • Adds consistent focus-visible ring and hover effects
  • Integrates an accessible tooltip that appears on hover and focus (not relying on title)
    would DRY this up and keep a11y consistent across the app.

Sketch:

type IconButtonProps = {
  icon: React.ElementType;
  label: string;            // used for aria-label
  tone?: 'danger' | 'info'; // controls colors
  title?: string;           // for hover tooltip text
  onClick?: React.MouseEventHandler<HTMLButtonElement>;
  className?: string;
};

For tooltips, prefer an a11y-friendly solution that shows on focus and hover (e.g., Radix UI Tooltip or a small custom tooltip using aria-describedby) instead of title alone, which isn’t reliably accessible for keyboard-only users.

Also applies to: 160-170, 172-179, 236-243


160-170: Optional: keep labels consistent.

Minor microcopy consistency: you’re using “Edit task”, “View task details”, and “Flip back to front”. Consider aligning tone/case across components (e.g., “Edit task”, “View details”, “Flip to front”) for polish.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 044b04f and 2b73d9e.

📒 Files selected for processing (3)
  • archon-ui-main/src/components/project-tasks/DocumentCard.tsx (2 hunks)
  • archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx (4 hunks)
  • archon-ui-main/src/components/project-tasks/TaskTableView.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • archon-ui-main/src/components/project-tasks/TaskTableView.tsx
  • archon-ui-main/src/components/project-tasks/DocumentCard.tsx
🧰 Additional context used
📓 Path-based instructions (2)
{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

{python/**/*.py,archon-ui-main/src/**/*.{ts,tsx,js,jsx}}: Remove dead code immediately; do not keep legacy/unused functions
Avoid comments that reference change history (e.g., LEGACY, CHANGED, REMOVED); keep comments focused on current functionality

Files:

  • archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx
archon-ui-main/src/components/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components in archon-ui-main/src/components/

Files:

  • archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx

Comment on lines +148 to 158
type="button"
onClick={(e) => {
e.stopPropagation();
onDelete(task);
}}
className="w-5 h-5 rounded-full flex items-center justify-center bg-red-100/80 dark:bg-red-500/20 text-red-600 dark:text-red-400 hover:bg-red-200 dark:hover:bg-red-500/30 hover:shadow-[0_0_10px_rgba(239,68,68,0.3)] transition-all duration-300"
title="Delete task"
aria-label="Delete task"
>
<Trash2 className="w-3 h-3" />
<Trash2 className="w-3 h-3" aria-hidden="true" />
</button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Repo-wide consistency check (buttons without explicit type or aria-hidden icons).

If you want quick verification that we didn’t miss any buttons lacking type="button" or icons missing aria-hidden in this folder, run:

Also applies to: 160-170, 172-179, 200-218, 236-243


🏁 Script executed:

#!/bin/bash
# From repo root
# 1) Buttons missing explicit type
rg -nP --type tsx -g 'archon-ui-main/src/**' '<button(?![^>]*\btype=)'

# 2) Icon components inside buttons lacking aria-hidden (heuristic for lucide-react icons)
rg -nP --type tsx -g 'archon-ui-main/src/**' '<button[\s\S]*?>[\s\S]*?<([A-Z][A-Za-z0-9]+)\b(?![^>]*aria-hidden)'

Length of output: 244


🏁 Script executed:

#!/bin/bash
# From repo root
# 1) Buttons missing explicit type attribute
rg -nP '<button(?![^>]*\btype=)' -g 'archon-ui-main/src/**/*.tsx'

# 2) Icon components inside buttons lacking aria-hidden (heuristic for lucide-react icons)
rg -nP '<button[\s\S]*?>[\s\S]*?<([A-Z][A-Za-z0-9]+)\b(?![^>]*aria-hidden)' -g 'archon-ui-main/src/**/*.tsx'

Length of output: 16718


🏁 Script executed:

#!/bin/bash
# Inspect DraggableTaskCard.tsx around the reported missing type attributes
sed -n '140,260p' archon-ui-main/src/components/project-tasks/DraggableTaskCard.tsx

Length of output: 6396


Ensure explicit type on all <button> elements

We ran a ripgrep scan across archon-ui-main/src/**/*.tsx and found numerous <button> tags missing a type attribute (which defaults to submit and can cause unintended form submissions). DraggableTaskCard is correctly using type="button", but many other components need updating.

Action items:

  • Add type="button" to every <button> that is not meant to submit or reset a form.
  • Reserve type="submit" / type="reset" only for actual form controls.
  • Icons inside buttons already include aria-hidden="true", so no missing aria-hidden issues were detected in our scan.

Examples of files needing a type attribute (non-exhaustive):

  • archon-ui-main/src/contexts/ToastContext.tsx:108
  • archon-ui-main/src/pages/SettingsPage.tsx:241
  • archon-ui-main/src/pages/MCPPage.tsx at lines 407, 421, 544, 554, 564, 574, 584, 594
  • archon-ui-main/src/components/ProjectCreationProgressCard.tsx:240
  • archon-ui-main/src/components/BackendStartupError.tsx:60

You can re-run:

rg -nP '<button(?![^>]*\btype=)' -g 'archon-ui-main/src/**/*.tsx'

to locate all occurrences and systematically add type="button".

@djankies

Copy link
Copy Markdown
Author

@Wirasm Hey there, thanks for taking a look. I've addressed the initial CR notes, however I didnt intend the scope of these changes to include accessibility changes. It looks like CodeRabbit wants me to take it in that direction after my latest push. I'm happy to do a more comprehensive accessibility review and make improvements in another PR. Is there anything specific you wanted me to remediate in this PR that isn't accessibility related?

@Wirasm

Wirasm commented Aug 25, 2025

Copy link
Copy Markdown
Collaborator

Looks great @djankies looking to merge today

@Wirasm Wirasm merged commit 1e3689c into coleam00:main Aug 25, 2025
8 checks passed
@djankies djankies deleted the feature/icon-button-tooltips branch September 22, 2025 05:32
coleam00 pushed a commit that referenced this pull request Apr 7, 2026
* feat: add per-node MCP servers for DAG workflows (#445)

Add `mcp: path/to/config.json` field to DAG workflow nodes. At execution
time, the executor reads the MCP config JSON, expands $VAR_NAME env
references in env/headers values, and passes the loaded servers to the
Claude Agent SDK via Options.mcpServers. MCP tool wildcards are auto-
added to allowedTools.

- Add mcp field to DagNodeBase type and loader validation
- Add mcpServers + allowedTools to WorkflowAssistantOptions and
  AssistantRequestOptions
- Pass mcpServers/allowedTools through ClaudeClient to SDK Options
- Handle system/init message to detect MCP connection failures
- Add Codex warning (per-node MCP not supported)
- Add Haiku warning (tool search not supported)
- Add MCP config path input in Web UI NodeInspector
- Add mcp to WorkflowCanvas reactFlowToDagNodes conversion
- Add 12 unit tests for loadMcpConfig and env var expansion

* fix: address review findings for per-node MCP servers

Type safety:
- Use SDK McpServerConfig type in AssistantRequestOptions (eliminates unsafe cast)
- Update WorkflowAssistantOptions.mcpServers to proper discriminated union
- Remove redundant cast in claude.ts

Error handling:
- Warn users when MCP config references undefined env vars
- Check safeSendMessage return value for MCP connection failures
- Check safeSendMessage return value for Haiku MCP warning
- Log unhandled system messages at debug level in both claude.ts and dag-executor.ts
- Coerce non-string env/header values with warning instead of silent passthrough

Code quality:
- Fix log event naming: dag_node_mcp_* → dag.mcp_* (domain.action_state format)
- Replace IIFE in loader mcp validation with plain if/else
- Extract duplicated env expansion logic into expandEnvVarsInRecord helper
- Merge split import from ./deps into single statement
- Return missingVars from loadMcpConfig/expandEnvVars for caller awareness

Documentation:
- Add mcp field to Node Fields table in docs/authoring-workflows.md
- Add mcp to DAG schema example, allowed_tools section, and summary list
- Add mcp to CLAUDE.md DAG feature list
- Add per-node MCP servers paragraph to README.md tool restrictions section

* docs: add MCP servers guide (docs/mcp-servers.md)

Comprehensive guide covering config file format (stdio/HTTP/SSE), env var
expansion, automatic tool wildcards, MCP-only nodes, connection failure
handling, workflow examples, troubleshooting, and popular server list.
Cross-referenced from authoring-workflows.md and CLAUDE.md.

* feat: add optional ntfy push notification to smart PR review

Add conditional notify node to archon-smart-pr-review workflow that sends
a push notification when the review completes. Gated behind a bash node
that checks for .archon/mcp/ntfy.json — silently skipped if not configured.

- Add check-ntfy bash node + notify MCP node to smart PR review workflow
- Add .archon/mcp/ to .gitignore (per-user MCP configs may contain secrets)
- Add "Push Notifications" setup guide to docs/mcp-servers.md
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…am00#688)

* feat: add per-node MCP servers for DAG workflows (coleam00#445)

Add `mcp: path/to/config.json` field to DAG workflow nodes. At execution
time, the executor reads the MCP config JSON, expands $VAR_NAME env
references in env/headers values, and passes the loaded servers to the
Claude Agent SDK via Options.mcpServers. MCP tool wildcards are auto-
added to allowedTools.

- Add mcp field to DagNodeBase type and loader validation
- Add mcpServers + allowedTools to WorkflowAssistantOptions and
  AssistantRequestOptions
- Pass mcpServers/allowedTools through ClaudeClient to SDK Options
- Handle system/init message to detect MCP connection failures
- Add Codex warning (per-node MCP not supported)
- Add Haiku warning (tool search not supported)
- Add MCP config path input in Web UI NodeInspector
- Add mcp to WorkflowCanvas reactFlowToDagNodes conversion
- Add 12 unit tests for loadMcpConfig and env var expansion

* fix: address review findings for per-node MCP servers

Type safety:
- Use SDK McpServerConfig type in AssistantRequestOptions (eliminates unsafe cast)
- Update WorkflowAssistantOptions.mcpServers to proper discriminated union
- Remove redundant cast in claude.ts

Error handling:
- Warn users when MCP config references undefined env vars
- Check safeSendMessage return value for MCP connection failures
- Check safeSendMessage return value for Haiku MCP warning
- Log unhandled system messages at debug level in both claude.ts and dag-executor.ts
- Coerce non-string env/header values with warning instead of silent passthrough

Code quality:
- Fix log event naming: dag_node_mcp_* → dag.mcp_* (domain.action_state format)
- Replace IIFE in loader mcp validation with plain if/else
- Extract duplicated env expansion logic into expandEnvVarsInRecord helper
- Merge split import from ./deps into single statement
- Return missingVars from loadMcpConfig/expandEnvVars for caller awareness

Documentation:
- Add mcp field to Node Fields table in docs/authoring-workflows.md
- Add mcp to DAG schema example, allowed_tools section, and summary list
- Add mcp to CLAUDE.md DAG feature list
- Add per-node MCP servers paragraph to README.md tool restrictions section

* docs: add MCP servers guide (docs/mcp-servers.md)

Comprehensive guide covering config file format (stdio/HTTP/SSE), env var
expansion, automatic tool wildcards, MCP-only nodes, connection failure
handling, workflow examples, troubleshooting, and popular server list.
Cross-referenced from authoring-workflows.md and CLAUDE.md.

* feat: add optional ntfy push notification to smart PR review

Add conditional notify node to archon-smart-pr-review workflow that sends
a push notification when the review completes. Gated behind a bash node
that checks for .archon/mcp/ntfy.json — silently skipped if not configured.

- Add check-ntfy bash node + notify MCP node to smart PR review workflow
- Add .archon/mcp/ to .gitignore (per-user MCP configs may contain secrets)
- Add "Push Notifications" setup guide to docs/mcp-servers.md
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…am00#688)

* feat: add per-node MCP servers for DAG workflows (coleam00#445)

Add `mcp: path/to/config.json` field to DAG workflow nodes. At execution
time, the executor reads the MCP config JSON, expands $VAR_NAME env
references in env/headers values, and passes the loaded servers to the
Claude Agent SDK via Options.mcpServers. MCP tool wildcards are auto-
added to allowedTools.

- Add mcp field to DagNodeBase type and loader validation
- Add mcpServers + allowedTools to WorkflowAssistantOptions and
  AssistantRequestOptions
- Pass mcpServers/allowedTools through ClaudeClient to SDK Options
- Handle system/init message to detect MCP connection failures
- Add Codex warning (per-node MCP not supported)
- Add Haiku warning (tool search not supported)
- Add MCP config path input in Web UI NodeInspector
- Add mcp to WorkflowCanvas reactFlowToDagNodes conversion
- Add 12 unit tests for loadMcpConfig and env var expansion

* fix: address review findings for per-node MCP servers

Type safety:
- Use SDK McpServerConfig type in AssistantRequestOptions (eliminates unsafe cast)
- Update WorkflowAssistantOptions.mcpServers to proper discriminated union
- Remove redundant cast in claude.ts

Error handling:
- Warn users when MCP config references undefined env vars
- Check safeSendMessage return value for MCP connection failures
- Check safeSendMessage return value for Haiku MCP warning
- Log unhandled system messages at debug level in both claude.ts and dag-executor.ts
- Coerce non-string env/header values with warning instead of silent passthrough

Code quality:
- Fix log event naming: dag_node_mcp_* → dag.mcp_* (domain.action_state format)
- Replace IIFE in loader mcp validation with plain if/else
- Extract duplicated env expansion logic into expandEnvVarsInRecord helper
- Merge split import from ./deps into single statement
- Return missingVars from loadMcpConfig/expandEnvVars for caller awareness

Documentation:
- Add mcp field to Node Fields table in docs/authoring-workflows.md
- Add mcp to DAG schema example, allowed_tools section, and summary list
- Add mcp to CLAUDE.md DAG feature list
- Add per-node MCP servers paragraph to README.md tool restrictions section

* docs: add MCP servers guide (docs/mcp-servers.md)

Comprehensive guide covering config file format (stdio/HTTP/SSE), env var
expansion, automatic tool wildcards, MCP-only nodes, connection failure
handling, workflow examples, troubleshooting, and popular server list.
Cross-referenced from authoring-workflows.md and CLAUDE.md.

* feat: add optional ntfy push notification to smart PR review

Add conditional notify node to archon-smart-pr-review workflow that sends
a push notification when the review completes. Gated behind a bash node
that checks for .archon/mcp/ntfy.json — silently skipped if not configured.

- Add check-ntfy bash node + notify MCP node to smart PR review workflow
- Add .archon/mcp/ to .gitignore (per-user MCP configs may contain secrets)
- Add "Push Notifications" setup guide to docs/mcp-servers.md
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.

2 participants