Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .jules/palette.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
## 2024-06-13 - Initial Setup\n**Learning:** Started logging UX enhancements.\n**Action:** Will log critical a11y/UX learnings here.
6 changes: 6 additions & 0 deletions patch_chatpanel.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/bash

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These temporary shell scripts (patch_chatpanel.sh, patch_chatpanel2.sh, patch_chatpanel3.sh, patch_nodeeditor.sh) appear to be automation artifacts used to apply changes to the codebase. They should not be committed to the repository as they pollute the project. Please remove them before merging.

sed -i 's/className="chat-bubble-edit-btn cancel"/className="chat-bubble-edit-btn cancel"\n aria-label="Cancel edit"/g' ui/components/ChatPanel.tsx
sed -i 's/className="chat-bubble-edit-btn save"/className="chat-bubble-edit-btn save"\n aria-label="Save edit"/g' ui/components/ChatPanel.tsx
sed -i 's/className="chat-show-more-btn"/className="chat-show-more-btn"\n aria-label={isCollapsed ? "Show more message content" : "Show less message content"}/g' ui/components/ChatPanel.tsx
sed -i 's/className="chat-action-btn"/className="chat-action-btn"\n aria-label={isCopied ? "Copied" : "Copy message"}/g' ui/components/ChatPanel.tsx
# Need more precise patching for aria labels based on title props
4 changes: 4 additions & 0 deletions patch_chatpanel2.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/bash
sed -i 's/title="Copy message"/title="Copy message"\n aria-label={isCopied ? "Copied message" : "Copy message"}/g' ui/components/ChatPanel.tsx
sed -i 's/title="Retry response"/title="Retry response"\n aria-label="Retry response"/g' ui/components/ChatPanel.tsx
sed -i 's/title="Edit message"/title="Edit message"\n aria-label="Edit message"/g' ui/components/ChatPanel.tsx
4 changes: 4 additions & 0 deletions patch_chatpanel3.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/bash
sed -i 's/className="chat-bubble-edit-btn cancel"/className="chat-bubble-edit-btn cancel"\n aria-label="Cancel edit"/g' ui/components/ChatPanel.tsx
sed -i 's/className="chat-bubble-edit-btn save"/className="chat-bubble-edit-btn save"\n aria-label="Save edit"/g' ui/components/ChatPanel.tsx
sed -i 's/className="chat-show-more-btn"/className="chat-show-more-btn"\n aria-label={isCollapsed ? "Show more message content" : "Show less message content"}/g' ui/components/ChatPanel.tsx
3 changes: 3 additions & 0 deletions patch_nodeeditor.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/bash
sed -i 's/title="Toggle interactive charts render workspace assets"/title="Toggle interactive charts render workspace assets"\n aria-label="Toggle interactive charts"/g' ui/components/NodeEditorDetail.tsx
sed -i 's/title="Expand editor to full center canvas focus"/title="Expand editor to full center canvas focus"\n aria-label="Expand editor"/g' ui/components/NodeEditorDetail.tsx
10 changes: 10 additions & 0 deletions ui/components/ChatPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,15 @@ const ChatMessageBubble = React.memo(function ChatMessageBubble({
<button
type="button"
className="chat-bubble-edit-btn cancel"
aria-label="Cancel edit"
onClick={onCancelEdit}
Comment on lines 116 to 118

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The button has visible text "Cancel". Adding aria-label="Cancel edit" is redundant and overrides the visible text, violating WCAG 2.5.3 (Label in Name). The aria-label should be removed.

Suggested change
className="chat-bubble-edit-btn cancel"
aria-label="Cancel edit"
onClick={onCancelEdit}
className="chat-bubble-edit-btn cancel"
onClick={onCancelEdit}

>
Cancel
</button>
<button
type="button"
className="chat-bubble-edit-btn save"
aria-label="Save edit"
onClick={() => void onSaveEdit(index, editingContent)}
Comment on lines 124 to 126

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The button has visible text "Save". Adding aria-label="Save edit" is redundant and overrides the visible text, violating WCAG 2.5.3 (Label in Name). The aria-label should be removed.

Suggested change
className="chat-bubble-edit-btn save"
aria-label="Save edit"
onClick={() => void onSaveEdit(index, editingContent)}
className="chat-bubble-edit-btn save"
onClick={() => void onSaveEdit(index, editingContent)}

>
Save
Expand All @@ -147,6 +149,9 @@ const ChatMessageBubble = React.memo(function ChatMessageBubble({
<button
type="button"
className="chat-show-more-btn"
aria-label={
isCollapsed ? "Show more message content" : "Show less message content"
}
onClick={() => setIsCollapsed(!isCollapsed)}
>
{isCollapsed ? "Show more" : "Show less"}
Expand All @@ -163,6 +168,7 @@ const ChatMessageBubble = React.memo(function ChatMessageBubble({
className="chat-action-btn"
onClick={() => onCopyMessage(message.content, message.id)}
title="Copy message"
aria-label={isCopied ? "Copied message" : "Copy message"}
Comment on lines 170 to +171

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When the message is copied (isCopied is true), the aria-label updates to "Copied message", but the title attribute remains static as "Copy message". This creates a mismatch between the visual tooltip (which still says "Copy message" on hover) and the screen reader announcement. The title attribute should be made dynamic to match the copied state, similar to how it is done in markdownUtils.tsx.

Suggested change
title="Copy message"
aria-label={isCopied ? "Copied message" : "Copy message"}
title={isCopied ? "Copied!" : "Copy message"}
aria-label={isCopied ? "Copied message" : "Copy message"}

>
{isCopied ? (
<svg
Expand Down Expand Up @@ -198,6 +204,7 @@ const ChatMessageBubble = React.memo(function ChatMessageBubble({
className="chat-action-btn"
onClick={() => onRetryMessage(index)}
title="Retry response"
aria-label="Retry response"
>
<svg
width="15"
Expand All @@ -222,6 +229,7 @@ const ChatMessageBubble = React.memo(function ChatMessageBubble({
className="chat-action-btn"
onClick={() => onCopyMessage(message.content, message.id)}
title="Copy message"
aria-label={isCopied ? "Copied message" : "Copy message"}
Comment on lines 231 to +232

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When the message is copied (isCopied is true), the aria-label updates to "Copied message", but the title attribute remains static as "Copy message". This creates a mismatch between the visual tooltip (which still says "Copy message" on hover) and the screen reader announcement. The title attribute should be made dynamic to match the copied state, similar to how it is done in markdownUtils.tsx.

Suggested change
title="Copy message"
aria-label={isCopied ? "Copied message" : "Copy message"}
title={isCopied ? "Copied!" : "Copy message"}
aria-label={isCopied ? "Copied message" : "Copy message"}

>
{isCopied ? (
<svg
Expand Down Expand Up @@ -257,6 +265,7 @@ const ChatMessageBubble = React.memo(function ChatMessageBubble({
className="chat-action-btn"
onClick={() => onStartEdit(message.id, message.content)}
title="Edit message"
aria-label="Edit message"
>
<svg
width="15"
Expand All @@ -277,6 +286,7 @@ const ChatMessageBubble = React.memo(function ChatMessageBubble({
className="chat-action-btn"
onClick={() => onRetryMessage(index)}
title="Retry response"
aria-label="Retry response"
>
<svg
width="15"
Expand Down
2 changes: 2 additions & 0 deletions ui/components/NodeEditorDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ export default function NodeEditorDetail({
className={`charts-toggle-btn ${chartsEnabled ? "active" : ""}`}
onClick={() => setNodeEditorChartsEnabled(!chartsEnabled)}
title="Toggle interactive charts render workspace assets"
aria-label="Toggle interactive charts"
>
Comment on lines 196 to 198

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The button already has clear, visible text: πŸ“Š Charts: {chartsEnabled ? "ON" : "OFF"}. Adding aria-label="Toggle interactive charts" completely overrides the button's text content for screen readers. As a result, screen reader users will only hear "Toggle interactive charts" and will not be able to hear whether the charts are currently "ON" or "OFF". It is best to remove the aria-label attribute entirely since the visible text is already fully accessible and descriptive.

Suggested change
title="Toggle interactive charts render workspace assets"
aria-label="Toggle interactive charts"
>
title="Toggle interactive charts render workspace assets"
>

πŸ“Š Charts: {chartsEnabled ? "ON" : "OFF"}
</button>
Expand All @@ -203,6 +204,7 @@ export default function NodeEditorDetail({
className="detail-expand-btn"
onClick={onExpand}
title="Expand editor to full center canvas focus"
aria-label="Expand editor"
>
Comment on lines 206 to 208

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The button has visible text "Focus Canvas". Adding aria-label="Expand editor" overrides this text, violating WCAG 2.1 Success Criterion 2.5.3 (Label in Name), which requires that the accessible name of a control contains its visible text. This mismatch breaks voice control activation (e.g., users saying "Click Focus Canvas" will fail because the accessible name is "Expand editor"). Since the visible text is already fully accessible, the aria-label should be removed.

Suggested change
title="Expand editor to full center canvas focus"
aria-label="Expand editor"
>
title="Expand editor to full center canvas focus"
>

<span className="expand-icon">β›Ά</span> Focus Canvas
</button>
Expand Down