Skip to content

🔖 style(Bookmarks): Simplify card actions and consolidate delete functionality#11586

Draft
berry-13 wants to merge 2 commits intodevfrom
style/bookmark-ui
Draft

🔖 style(Bookmarks): Simplify card actions and consolidate delete functionality#11586
berry-13 wants to merge 2 commits intodevfrom
style/bookmark-ui

Conversation

@berry-13
Copy link
Collaborator

@berry-13 berry-13 commented Jan 31, 2026

Summary

Removes standalone DeleteBookmarkButton and EditBookmarkButton components in favor of integrated actions within BookmarkCardActions. Streamlines the bookmark management UI with a more cohesive interaction pattern. Updates translations for bookmark deletion confirmation dialog. Reduces component fragmentation and improves maintainability

Change Type

  • Refactor
  • Style

Testing

  • Bookmark card displays edit and delete actions
  • Edit bookmark opens edit dialog
  • Delete bookmark shows confirmation dialog
  • Confirm delete removes bookmark successfully
  • Cancel delete closes dialog without action
  • Bookmark list updates after deletion
  • Actions are keyboard accessible

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes
  • Any changes dependent on mine have been merged and published in downstream modules.

@berry-13 berry-13 added ♿ a11y Accessibility 🎨 design UI/UX improvements 🔨 refactor Code cleanup without new features labels Jan 31, 2026
@berry-13 berry-13 marked this pull request as draft February 1, 2026 10:24
@danny-avila danny-avila requested a review from Copilot February 4, 2026 12:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the bookmarks side panel UI by consolidating edit/delete actions into BookmarkCardActions, simplifying the component structure and improving accessibility text.

Changes:

  • Removes standalone DeleteBookmarkButton and EditBookmarkButton components and updates exports accordingly.
  • Updates bookmark action UI to use a single consolidated actions component with updated delete confirmation content.
  • Adds/adjusts English i18n strings to support more descriptive ARIA labels and screen-reader text.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
client/src/locales/en/translation.json Updates delete confirmation copy and adds new bookmark-related ARIA/screen-reader strings.
client/src/components/SidePanel/Bookmarks/BookmarkTable.tsx Moves “new bookmark” accessible label to the tooltip anchor for the create button trigger.
client/src/components/SidePanel/Bookmarks/BookmarkCardActions.tsx Consolidates card edit/delete actions and updates delete dialog content and labeling.
client/src/components/SidePanel/Bookmarks/BookmarkCard.tsx Improves count badge accessibility with sr-only text for screen readers.
client/src/components/Bookmarks/index.ts Removes exports for deleted standalone bookmark action components and reorders remaining exports.
client/src/components/Bookmarks/EditBookmarkButton.tsx Deleted (standalone edit button component removed).
client/src/components/Bookmarks/DeleteBookmarkButton.tsx Deleted (standalone delete button component removed).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +106 to 110
selection={
<Button onClick={handleDelete} variant="destructive">
{isDeleting ? <Spinner /> : localize('com_ui_delete')}
</Button>
}
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

OGDialogTemplate's selection prop expects an object ({ selectHandler, selectText, isLoading, ... }), not a JSX element. Passing a <Button /> here means the confirm-delete button won't render (and TypeScript should complain), so deletion can't be confirmed. Use selection={{ selectHandler: handleDelete, selectText: localize('com_ui_delete'), isLoading: isDeleting, selectClasses: ... }} or pass a custom confirm button via the buttons prop instead.

Suggested change
selection={
<Button onClick={handleDelete} variant="destructive">
{isDeleting ? <Spinner /> : localize('com_ui_delete')}
</Button>
}
selection={{
selectHandler: handleDelete,
selectText: localize('com_ui_delete'),
isLoading: isDeleting,
}}

Copilot uses AI. Check for mistakes.
"com_ui_bookmarks_create_exists": "This bookmark already exists",
"com_ui_bookmarks_create_success": "Bookmark created successfully",
"com_ui_bookmarks_delete": "Delete Bookmark",
"com_ui_bookmarks_count": "{{0}} conversations in {{1}}",
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

com_ui_bookmarks_count is always pluralized as "{{0}} conversations...", but bookmark.count can be 1, which will produce ungrammatical screen-reader text ("1 conversations"). Consider adding singular/plural variants (e.g., separate keys) or restructure the string so the code can reuse the existing com_ui_conversation vs com_ui_conversations logic.

Suggested change
"com_ui_bookmarks_count": "{{0}} conversations in {{1}}",
"com_ui_bookmarks_count": "{{0}} in {{1}}",

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

♿ a11y Accessibility 🎨 design UI/UX improvements 🔨 refactor Code cleanup without new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant