-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Remove org switcher tooltip #8269
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
Conversation
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.
Found issues that need attention.
| aria-label={selectedOrgId ? currentOrg?.organization.name : t("cloud:personalAccount")}> | ||
| {renderAccountIcon()} | ||
| </SelectTrigger> | ||
| <div className="inline-block ml-1"> |
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.
Accessibility concern: Removing the tooltip eliminates the "Switch account" text that provided context for what this component does. While the SelectTrigger still has an aria-label, removing the tooltip reduces discoverability for users who rely on hover tooltips to understand UI elements.
Consider keeping the tooltip for better UX, or ensure the aria-label provides sufficient context for screen reader users.
| import { Building2 } from "lucide-react" | ||
| import { Select, SelectContent, SelectItem, SelectTrigger, SelectSeparator } from "@/components/ui/select" | ||
| import { StandardTooltip } from "@src/components/ui" | ||
| import { useAppTranslation } from "@src/i18n/TranslationContext" |
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.
Unused import: After removing the StandardTooltip wrapper, this import is no longer needed and should be removed.
Also, the translation key is now unused anywhere in the codebase and should be removed from all translation files to avoid dead code.
| {renderAccountIcon()} | ||
| </SelectTrigger> | ||
| <div className="inline-block ml-1"> | ||
| <Select value={currentValue} onValueChange={handleOrganizationChange} disabled={isLoading}> |
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.
UX consistency: The codebase extensively uses StandardTooltip (224 occurrences) to provide contextual information for interactive elements. Removing it from this component breaks the established UX pattern.
Consider if this change aligns with the overall UX strategy, as tooltips help users understand what UI elements do without having to interact with them first.
Important
Remove tooltip from
CloudAccountSwitchercomponent, simplifying the UI.StandardTooltipfromCloudAccountSwitcherinCloudAccountSwitcher.tsx, eliminating the tooltip on the account switcher.StandardTooltipfromCloudAccountSwitcher.tsx.This description was created by
for 3564ba3. You can customize this summary. It will automatically update as commits are pushed.