[Fix] Improve toast notification behavior#488
Conversation
Signed-off-by: samzong <[email protected]> ## Considered and deferred - packages/desktop/src/renderer/lib/toast.ts:14 [BOT-SCOPE]: Existing renderer controls use i18n for visible aria labels, but Sonner toast options are module-level constants; moving this into a translated component/builder is broader than this fix. - packages/desktop/src/renderer/lib/toast.ts:29 [BOT-NIT]: `Object.assign` preserves the Sonner API surface while overriding warning/error defaults; replacing it with a hand-written facade would recreate API drift risk. Signed-off-by: samzong <[email protected]>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the application's toast notification system to improve user experience and UI consistency. By centralizing toast configuration through a new renderer facade, the changes ensure uniform behavior for success, warning, and error notifications while resolving layout conflicts with existing UI elements. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Hi @samzong, DetailsInstructions for interacting with me using comments are available here. |
|
/auto-cc |
💡 Codex Review Localize the close-button label
For non-English locales, this new toast dismiss control remains in English because the aria label is hardcoded in the renderer facade. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request centralizes toast notification management by creating a dedicated utility in lib/toast.ts and introducing an AppToaster component to standardize styles and durations across the application. The review feedback identifies a violation of internationalization standards (Rule 43), specifically pointing out that user-facing strings like closeButtonAriaLabel should be translated using the t() function within a React component rather than being defined as module-level constants.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/desktop/src/renderer/lib/toast.ts (14)
The closeButtonAriaLabel is a user-facing string and should be internationalized using t() as per the repository style guide (rule 43). Since TOAST_OPTIONS is a module-level constant, you should move this label definition to the AppToaster component in App.tsx where the useTranslation hook is available.
References
- Rule 43: t() for all user-facing strings. (link)
packages/desktop/src/renderer/App.tsx (1-3)
Import useTranslation to support internationalization of user-facing strings in the AppToaster component.
import { useState, useEffect, useCallback } from 'react';
import { useTranslation } from 'react-i18next';
import { motion, AnimatePresence } from 'framer-motion';
import { Toaster, TOAST_DURATION_MS, TOAST_OPTIONS } from '@/lib/toast';
References
- Rule 43: t() for all user-facing strings. (link)
packages/desktop/src/renderer/App.tsx (23-34)
Use the useTranslation hook to provide a translated aria-label for the toast close button, adhering to the internationalization requirements in the style guide.
function AppToaster({ themeMode }: { themeMode: Theme }) {
const { t } = useTranslation();
return (
<Toaster
theme={themeMode === 'auto' ? 'system' : themeMode}
position="top-right"
closeButton
visibleToasts={3}
duration={TOAST_DURATION_MS}
toastOptions={TOAST_OPTIONS}
closeButtonAriaLabel={t('common.close')}
/>
);
}
References
- Rule 43: t() for all user-facing strings. (link)
1 participant
Summary
Move renderer toast notifications away from the bottom action area, make them dismissible, and centralize toast behavior through a renderer facade.
Type of change
[Feat]new feature[Fix]bug fix[UI]UI or UX change[Docs]documentation-only change[Refactor]internal cleanup[Build]CI, packaging, or tooling change[Chore]maintenanceWhy is this needed?
Toast notifications were appearing in the bottom-right corner, where they can overlap common save, exit, confirmation, and input actions. The dismiss control also needed to read as part of the toast instead of a floating window control.
What changed?
@/lib/toastas the renderer toast facade with default success/info, longer warning, and finite error durations.Architecture impact
docs/architecture-invariants.md: renderer remains presentation-only; no main/preload/core boundaries changed.Linked issues
Closes #
Validation
pnpm lintpnpm testpnpm buildpnpm check:ui-contractCommands, screenshots, or notes:
Screenshots or recordings
Manual screenshot review confirmed the default floating close button looked wrong; this PR moves the close button inside the toast card while preserving existing color tokens:
var(--bg-elevated),var(--border),var(--text-primary),var(--text-muted), andvar(--bg-hover).Release note
NONE.Checklist
git commit -s)[Feat],[Fix],[UI],[Docs],[Refactor],[Build], or[Chore]Considered and deferred
Object.assignpreserves the Sonner API surface while overriding warning/error defaults; replacing it with a hand-written facade would recreate API drift risk.