-
Notifications
You must be signed in to change notification settings - Fork 6
Improve the Post cancellation dialog buttons #9031
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThis PR updates English i18n translation keys (renaming Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/core/i18n/en/translation.en.json(2 hunks)src/core/ui/dialogs/ConfirmationDialog.tsx(3 hunks)src/domain/collaboration/callout/CalloutDialogs/CreateCalloutDialog.tsx(1 hunks)src/domain/collaboration/callout/CalloutDialogs/EditCalloutDialog.tsx(1 hunks)src/domain/collaboration/calloutContributions/post/CalloutContributionDialogPost.tsx(1 hunks)src/domain/collaboration/post/PostCreationDialog/PostCreationDialog.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*
⚙️ CodeRabbit configuration file
**/*: Repository overview:
- React 19 + TypeScript single-page app for the Alkemio platform. Uses Vite 5, Apollo Client 3, MUI 7, Emotion, Vitest.
- Large surface (production build transforms ~18k modules). Core implementation lives in src/core, src/domain, and src/main; generated GraphQL hooks reside in src/core/apollo/generated.
- Authoritative docs: README.md, docs/development-setup.md, .specify/memory/constitution.md, agents.md, .github/copilot-instructions.md.
Environment & bootstrap:
- Ensure Node >=20.9.0 (Volta pins 20.15.1) and pnpm >=10.17.1.
- Keep .env (checked in) present; override locally via .env.local.
pnpm install(validated 2025-10-30, ~1s with cached deps). pnpm warns about ignored build scripts (@sentry/cli, esbuild, husky, msw); ignore or runpnpm approve-buildsif required.Validation flow (validated 2025-10-30):
pnpm lint→ runstsc --noEmitthen ESLint onsrc/**/*.ts(x); completed cleanly.pnpm vitest run --reporter=basic.pnpm build→Rewrites.build/docker/.env.baseandpublic/env-config.js; revert before commit. Emits expected large-chunk warnings.pnpm serve:dev→ serves./buildon port 3001 (afterpnpm build).pnpm format→ Prettier forsrc/**/*.ts(x).pnpm codegen→ requires backend GraphQL endpoint athttp://localhost:3000/graphql; regeneratessrc/core/apollo/generatedartifacts.pnpm start→ Vite dev server (app reachable via Traefik onhttp://localhost:3000, Vite listens on 3001). Regeneratespublic/env-config.jseach run.Layout & architecture:
src/index.tsxbootstraps the SPA;src/root.tsxwires Sentry, Apollo, routing, auth, and global state.src/core/houses shared infrastructure (auth, analytics, routing helpers, UI kit, utilities).src/domain/contains domain façades/hooks grouped by business context (space, innovationHub, communication, etc.).src/main/provides routed shells, layouts, notifications, admin flows; keep business rules in `src/domain...
Files:
src/domain/collaboration/post/PostCreationDialog/PostCreationDialog.tsxsrc/core/ui/dialogs/ConfirmationDialog.tsxsrc/domain/collaboration/callout/CalloutDialogs/EditCalloutDialog.tsxsrc/domain/collaboration/calloutContributions/post/CalloutContributionDialogPost.tsxsrc/domain/collaboration/callout/CalloutDialogs/CreateCalloutDialog.tsxsrc/core/i18n/en/translation.en.json
src/**/*.{ts,tsx,js}
⚙️ CodeRabbit configuration file
src/**/*.{ts,tsx,js}: Review the React/TypeScript code for correctness and alignment with docs/code-guidelines.md.
Rely on global error handling and logging; only flag missing local try/catch or logging when additional mitigation is required.
Non-null assertions are discouraged except when paired with skip guards on GraphQL queries (see existing patterns).
Watch for security issues (XSS, CSRF, sensitive data exposure) and TypeScript typing violations.
Files:
src/domain/collaboration/post/PostCreationDialog/PostCreationDialog.tsxsrc/core/ui/dialogs/ConfirmationDialog.tsxsrc/domain/collaboration/callout/CalloutDialogs/EditCalloutDialog.tsxsrc/domain/collaboration/calloutContributions/post/CalloutContributionDialogPost.tsxsrc/domain/collaboration/callout/CalloutDialogs/CreateCalloutDialog.tsx
src/**/*.json
⚙️ CodeRabbit configuration file
src/**/*.json: Validate structure, schema compliance, and absence of secrets. Keep localization payloads consistent.
Files:
src/core/i18n/en/translation.en.json
🧠 Learnings (2)
📚 Learning: 2025-10-07T10:50:35.561Z
Learnt from: ccanos
Repo: alkem-io/client-web PR: 8774
File: src/core/i18n/en/translation.en.json:881-889
Timestamp: 2025-10-07T10:50:35.561Z
Learning: In src/core/i18n/en/translation.en.json, the keys callout.contributions.contributionsItemsCountExpand and callout.contributions.contributionsCollapse are always used in PaginationExpander (src/domain/collaboration/calloutContributions/contributionsCardsExpandable/PaginationExpander.tsx) only when totalContributions > pageSize (typically >= 4). Therefore, they should remain plural-only (no _one/_other variants needed).
Applied to files:
src/domain/collaboration/post/PostCreationDialog/PostCreationDialog.tsxsrc/domain/collaboration/callout/CalloutDialogs/EditCalloutDialog.tsxsrc/domain/collaboration/calloutContributions/post/CalloutContributionDialogPost.tsxsrc/domain/collaboration/callout/CalloutDialogs/CreateCalloutDialog.tsxsrc/core/i18n/en/translation.en.json
📚 Learning: 2024-12-04T12:56:29.595Z
Learnt from: bobbykolev
Repo: alkem-io/client-web PR: 7278
File: src/domain/collaboration/callout/creationDialog/CalloutCreationDialog.tsx:154-154
Timestamp: 2024-12-04T12:56:29.595Z
Learning: When implementing `scrollToTop()` in `CalloutCreationDialog.tsx`, ensure it is only called after a successful callout creation and not executed in the event of an error.
Applied to files:
src/domain/collaboration/callout/CalloutDialogs/EditCalloutDialog.tsxsrc/domain/collaboration/calloutContributions/post/CalloutContributionDialogPost.tsxsrc/domain/collaboration/callout/CalloutDialogs/CreateCalloutDialog.tsx
🔇 Additional comments (8)
src/domain/collaboration/callout/CalloutDialogs/CreateCalloutDialog.tsx (1)
269-269: LGTM! Translation key updated to camelCase convention.The rename from
buttons.yes-closetobuttons.yesClosealigns with the broader i18n key standardization across the codebase. The translation value remains unchanged.src/domain/collaboration/post/PostCreationDialog/PostCreationDialog.tsx (1)
116-119: LGTM! Improved dialog semantics for post creation cancellation.The shift from
closeConfirmtocancelConfirmkeys provides more appropriate messaging for a creation dialog where users are abandoning unsaved work. The addition ofcancelButtonTextIdenables explicit control over the cancel button label, enhancing UX clarity.src/domain/collaboration/callout/CalloutDialogs/EditCalloutDialog.tsx (1)
227-227: LGTM! Consistent i18n key update.The rename to
buttons.yesClosemaintains consistency with the naming convention applied throughout this PR.src/domain/collaboration/calloutContributions/post/CalloutContributionDialogPost.tsx (1)
256-259: LGTM! Dialog updated with cancel-focused translations.The shift to
cancelConfirmkeys and addition ofcancelButtonTextIdprovides clearer messaging when users abandon post edits with unsaved changes. Consistent with similar dialogs across the codebase.src/core/ui/dialogs/ConfirmationDialog.tsx (2)
19-19: LGTM! New optional prop for i18n cancel button text.The
cancelButtonTextIdprop follows the same pattern asconfirmButtonTextIdand maintains backward compatibility by being optional.
45-47: LGTM! Cancel button text derivation consistent with confirm button pattern.The logic properly translates
cancelButtonTextIdwhen provided and falls back tocancelButtonText, maintaining consistency with the existingconfirmButtonTextimplementation.src/core/i18n/en/translation.en.json (2)
126-127: LGTM! Button translations updated with improved naming convention.The rename to
yesClose(camelCase) improves consistency with other button keys likeyesContinueandyesReplace. The newyesCancelkey provides semantic distinction between closing and canceling operations.
3540-3543: New cancel-focused translations added alongside close-focused ones.The new
cancelConfirmtranslations provide more appropriate language for abandoning unsaved work ("Are you sure you want to cancel this post?"). Note thatcloseConfirm(lines 3536-3538) is still present, suggesting both are intentionally maintained for different dialog contexts.This semantic distinction is valuable:
cancelConfirm: For abandoning in-progress workcloseConfirm: For closing completed/saved work
| <Button variant="text" onClick={actions.onCancel}> | ||
| {cancelButtonText || t('buttons.cancel')} | ||
| </Button> | ||
| {Boolean(actions.onConfirm) && ( | ||
| <Button variant="text" loading={state?.isLoading} disabled={state?.isLoading} onClick={actions.onConfirm}> | ||
| <Button | ||
| variant="contained" | ||
| loading={state?.isLoading} | ||
| disabled={state?.isLoading} | ||
| onClick={actions.onConfirm} | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify button styling works well for destructive confirmations.
The button variant swap (cancel: text, confirm: contained) emphasizes the confirm action, which is a common UX pattern. However, for destructive confirmations (e.g., delete dialogs), this could make dangerous actions more prominent.
Please verify this change works appropriately across all dialog types, especially destructive ones. Consider whether destructive confirmations might need different styling.
Run the following script to identify confirmation dialogs used for destructive actions:
Additionally, the explicit loading and disabled props on the confirm button are good improvements for state handling.
🏁 Script executed:
#!/bin/bash
# Description: Find ConfirmationDialog usages with destructive actions
# Search for ConfirmationDialog with delete/remove/destroy keywords
rg -n -B5 -A10 'ConfirmationDialog' --type=tsx --type=ts | rg -i 'delete|remove|destroy|destructive'
# Search for confirmButtonTextId with delete/remove
rg -n "confirmButtonTextId.*['\"]buttons\\.(delete|remove|yes.*delete)" --type=tsx --type=tsLength of output: 55
🏁 Script executed:
#!/bin/bash
# Search for ConfirmationDialog usages with destructive patterns (no --type flags)
echo "=== Searching for ConfirmationDialog imports and destructive action patterns ==="
rg -n 'ConfirmationDialog' -A 15 | head -100
echo ""
echo "=== Searching for delete/remove button text patterns ==="
rg -n "confirmButtonTextId|confirm.*text.*delete|remove|destroy" -i | head -50Length of output: 15557
🏁 Script executed:
#!/bin/bash
# Find all files that import or use ConfirmationDialog
echo "=== Files using ConfirmationDialog ==="
rg -l 'ConfirmationDialog' | head -20
echo ""
echo "=== Check src/domain for dialog patterns ==="
fd 'Dialog' src/domain --type f | head -20Length of output: 3269
🏁 Script executed:
#!/bin/bash
# Find and examine specific delete confirmation usages
echo "=== CalendarEventDetailView delete pattern ==="
rg -n -B5 -A15 "confirmButtonTextId: 'buttons.delete'" src/domain/timeline/calendar/views/CalendarEventDetailView.tsx
echo ""
echo "=== CalendarDialog delete pattern ==="
rg -n -B5 -A15 "confirmButtonTextId: 'buttons.delete'" src/domain/timeline/calendar/CalendarDialog.tsx
echo ""
echo "=== ChatWidgetInner clear chat dialog ==="
rg -n -B5 -A20 "handleClearChat\|openClearConfirm" src/main/guidance/chatWidget/ChatWidgetInner.tsx | head -80Length of output: 1731
🏁 Script executed:
#!/bin/bash
# Check the ConfirmationDialog component itself to see current state
echo "=== ConfirmationDialog implementation (lines 50-75) ==="
sed -n '50,75p' src/core/ui/dialogs/ConfirmationDialog.tsx
echo ""
echo "=== Check if any dialog overrides button styling ==="
rg -n "variant.*contained\|variant.*text" src/domain --type-list-all | head -30Length of output: 1383
Destructive confirmations need reduced prominence styling.
Verification found that destructive actions do use ConfirmationDialog: delete comment (CalendarEventDetailView.tsx:165), delete event (CalendarDialog.tsx:143), and clear chat (ChatWidgetInner.tsx). The styling change makes these dangerous actions more prominent via the contained variant, which violates UX best practice—destructive actions should be less, not more, prominent to avoid accidental activation.
The confirm button should use a warning/destructive styling (e.g., color="error" or a dedicated destructive variant) rather than the default contained style, particularly for delete and clear confirmations. Consider:
- Apply color="error" to confirm buttons when confirmButtonTextId contains 'delete', 'remove', or 'clear'
- Or create a destructive-specific dialog variant that uses warning styling
- Ensure dialogs with destructive actions (CalendarEventDetailView.tsx, CalendarDialog.tsx, ChatWidgetInner.tsx) are updated accordingly
Summary by CodeRabbit
New Features
Style
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.