-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,6 @@ | ||
| import { useState, useEffect } from "react" | ||
| 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" | ||
| import { vscode } from "@src/utils/vscode" | ||
| import { useExtensionState } from "@src/context/ExtensionStateContext" | ||
|
|
@@ -86,64 +85,62 @@ export const CloudAccountSwitcher = () => { | |
| } | ||
|
|
||
| return ( | ||
| <StandardTooltip content={t("cloud:switchAccount")}> | ||
| <div className="inline-block ml-1"> | ||
| <Select value={currentValue} onValueChange={handleOrganizationChange} disabled={isLoading}> | ||
| <SelectTrigger | ||
| className={cn( | ||
| "h-4.5 w-4.5 p-0 gap-0", | ||
| "bg-transparent opacity-90 hover:opacity-50", | ||
| "flex items-center justify-center", | ||
| "rounded-lg overflow-clip", | ||
| "border border-vscode-dropdown-border", | ||
| "[&>svg]:hidden", // Hide the default chevron/caret | ||
| isLoading && "opacity-50", | ||
| )} | ||
| aria-label={selectedOrgId ? currentOrg?.organization.name : t("cloud:personalAccount")}> | ||
| {renderAccountIcon()} | ||
| </SelectTrigger> | ||
| <div className="inline-block ml-1"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| <Select value={currentValue} onValueChange={handleOrganizationChange} disabled={isLoading}> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| <SelectTrigger | ||
| className={cn( | ||
| "h-4.5 w-4.5 p-0 gap-0", | ||
| "bg-transparent opacity-90 hover:opacity-50", | ||
| "flex items-center justify-center", | ||
| "rounded-lg overflow-clip", | ||
| "border border-vscode-dropdown-border", | ||
| "[&>svg]:hidden", // Hide the default chevron/caret | ||
| isLoading && "opacity-50", | ||
| )} | ||
| aria-label={selectedOrgId ? currentOrg?.organization.name : t("cloud:personalAccount")}> | ||
| {renderAccountIcon()} | ||
| </SelectTrigger> | ||
|
|
||
| <SelectContent> | ||
| {/* Personal Account Option */} | ||
| <SelectItem value="personal"> | ||
| <SelectContent> | ||
| {/* Personal Account Option */} | ||
| <SelectItem value="personal"> | ||
| <div className="flex items-center gap-2"> | ||
| {cloudUserInfo.picture ? ( | ||
| <img | ||
| src={cloudUserInfo.picture} | ||
| alt={cloudUserInfo.name || cloudUserInfo.email} | ||
| className="w-4.5 h-4.5 rounded-full object-cover overflow-clip" | ||
| /> | ||
| ) : ( | ||
| <div className="w-4.5 h-4.5 rounded-full flex items-center justify-center bg-vscode-button-background text-vscode-button-foreground text-xs"> | ||
| {cloudUserInfo.name?.charAt(0) || cloudUserInfo.email?.charAt(0) || "?"} | ||
| </div> | ||
| )} | ||
| <span>{t("cloud:personalAccount")}</span> | ||
| </div> | ||
| </SelectItem> | ||
|
|
||
| {cloudOrganizations.length > 0 && <SelectSeparator />} | ||
|
|
||
| {/* Organization Options */} | ||
| {cloudOrganizations.map((org) => ( | ||
| <SelectItem key={org.organization.id} value={org.organization.id}> | ||
| <div className="flex items-center gap-2"> | ||
| {cloudUserInfo.picture ? ( | ||
| {org.organization.image_url ? ( | ||
| <img | ||
| src={cloudUserInfo.picture} | ||
| alt={cloudUserInfo.name || cloudUserInfo.email} | ||
| src={org.organization.image_url} | ||
| alt="" | ||
| className="w-4.5 h-4.5 rounded-full object-cover overflow-clip" | ||
| /> | ||
| ) : ( | ||
| <div className="w-4.5 h-4.5 rounded-full flex items-center justify-center bg-vscode-button-background text-vscode-button-foreground text-xs"> | ||
| {cloudUserInfo.name?.charAt(0) || cloudUserInfo.email?.charAt(0) || "?"} | ||
| </div> | ||
| <Building2 className="w-4.5 h-4.5" /> | ||
| )} | ||
| <span>{t("cloud:personalAccount")}</span> | ||
| <span className="truncate">{org.organization.name}</span> | ||
| </div> | ||
| </SelectItem> | ||
|
|
||
| {cloudOrganizations.length > 0 && <SelectSeparator />} | ||
|
|
||
| {/* Organization Options */} | ||
| {cloudOrganizations.map((org) => ( | ||
| <SelectItem key={org.organization.id} value={org.organization.id}> | ||
| <div className="flex items-center gap-2"> | ||
| {org.organization.image_url ? ( | ||
| <img | ||
| src={org.organization.image_url} | ||
| alt="" | ||
| className="w-4.5 h-4.5 rounded-full object-cover overflow-clip" | ||
| /> | ||
| ) : ( | ||
| <Building2 className="w-4.5 h-4.5" /> | ||
| )} | ||
| <span className="truncate">{org.organization.name}</span> | ||
| </div> | ||
| </SelectItem> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </div> | ||
| </StandardTooltip> | ||
| ))} | ||
| </SelectContent> | ||
| </Select> | ||
| </div> | ||
| ) | ||
| } | ||
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.