Skip to content
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

Enhance Sidebar component accessibility #266

Merged
merged 2 commits into from
Apr 9, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 48 additions & 10 deletions client/src/components/Sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,19 @@ const Sidebar = ({
<div className="p-4 flex-1 overflow-auto">
<div className="space-y-4">
<div className="space-y-2">
<label className="text-sm font-medium">Transport Type</label>
<label
className="text-sm font-medium"
htmlFor="transport-type-select"
>
Transport Type
</label>
<Select
value={transportType}
onValueChange={(value: "stdio" | "sse") =>
setTransportType(value)
}
>
<SelectTrigger>
<SelectTrigger id="transport-type-select">
<SelectValue placeholder="Select transport type" />
</SelectTrigger>
<SelectContent>
Expand All @@ -123,17 +128,26 @@ const Sidebar = ({
{transportType === "stdio" ? (
<>
<div className="space-y-2">
<label className="text-sm font-medium">Command</label>
<label className="text-sm font-medium" htmlFor="command-input">
Command
</label>
<Input
id="command-input"
placeholder="Command"
value={command}
onChange={(e) => setCommand(e.target.value)}
className="font-mono"
/>
</div>
<div className="space-y-2">
<label className="text-sm font-medium">Arguments</label>
<label
className="text-sm font-medium"
htmlFor="arguments-input"
>
Arguments
</label>
<Input
id="arguments-input"
placeholder="Arguments (space-separated)"
value={args}
onChange={(e) => setArgs(e.target.value)}
Expand All @@ -144,8 +158,11 @@ const Sidebar = ({
) : (
<>
<div className="space-y-2">
<label className="text-sm font-medium">URL</label>
<label className="text-sm font-medium" htmlFor="sse-url-input">
URL
</label>
<Input
id="sse-url-input"
placeholder="URL"
value={sseUrl}
onChange={(e) => setSseUrl(e.target.value)}
Expand All @@ -157,6 +174,7 @@ const Sidebar = ({
variant="outline"
onClick={() => setShowBearerToken(!showBearerToken)}
className="flex items-center w-full"
aria-expanded={showBearerToken}
>
{showBearerToken ? (
<ChevronDown className="w-4 h-4 mr-2" />
Expand All @@ -167,8 +185,14 @@ const Sidebar = ({
</Button>
{showBearerToken && (
<div className="space-y-2">
<label className="text-sm font-medium">Bearer Token</label>
<label
className="text-sm font-medium"
htmlFor="bearer-token-input"
>
Bearer Token
</label>
<Input
id="bearer-token-input"
placeholder="Bearer Token"
value={bearerToken}
onChange={(e) => setBearerToken(e.target.value)}
Expand All @@ -187,6 +211,7 @@ const Sidebar = ({
onClick={() => setShowEnvVars(!showEnvVars)}
className="flex items-center w-full"
data-testid="env-vars-button"
aria-expanded={showEnvVars}
>
{showEnvVars ? (
<ChevronDown className="w-4 h-4 mr-2" />
Expand All @@ -201,6 +226,7 @@ const Sidebar = ({
<div key={idx} className="space-y-2 pb-4">
<div className="flex gap-2">
<Input
aria-label={`Environment variable key ${idx + 1}`}
placeholder="Key"
value={key}
onChange={(e) => {
Expand Down Expand Up @@ -243,6 +269,7 @@ const Sidebar = ({
</div>
<div className="flex gap-2">
<Input
aria-label={`Environment variable value ${idx + 1}`}
type={shownEnvVars.has(key) ? "text" : "password"}
placeholder="Value"
value={value}
Expand Down Expand Up @@ -309,6 +336,7 @@ const Sidebar = ({
onClick={() => setShowConfig(!showConfig)}
className="flex items-center w-full"
data-testid="config-button"
aria-expanded={showConfig}
>
{showConfig ? (
<ChevronDown className="w-4 h-4 mr-2" />
Expand All @@ -325,7 +353,10 @@ const Sidebar = ({
return (
<div key={key} className="space-y-2">
<div className="flex items-center gap-1">
<label className="text-sm font-medium text-green-600 break-all">
<label
className="text-sm font-medium text-green-600 break-all"
htmlFor={`${configKey}-input`}
>
{configItem.label}
</label>
<Tooltip>
Expand All @@ -339,6 +370,7 @@ const Sidebar = ({
</div>
{typeof configItem.value === "number" ? (
<Input
id={`${configKey}-input`}
type="number"
data-testid={`${configKey}-input`}
value={configItem.value}
Expand All @@ -365,7 +397,7 @@ const Sidebar = ({
setConfig(newConfig);
}}
>
<SelectTrigger>
<SelectTrigger id={`${configKey}-input`}>
<SelectValue />
</SelectTrigger>
<SelectContent>
Expand All @@ -375,6 +407,7 @@ const Sidebar = ({
</Select>
) : (
<Input
id={`${configKey}-input`}
data-testid={`${configKey}-input`}
value={configItem.value}
onChange={(e) => {
Expand Down Expand Up @@ -448,14 +481,19 @@ const Sidebar = ({

{loggingSupported && connectionStatus === "connected" && (
<div className="space-y-2">
<label className="text-sm font-medium">Logging Level</label>
<label
className="text-sm font-medium"
htmlFor="logging-level-select"
>
Logging Level
</label>
<Select
value={logLevel}
onValueChange={(value: LoggingLevel) =>
sendLogLevelRequest(value)
}
>
<SelectTrigger>
<SelectTrigger id="logging-level-select">
<SelectValue placeholder="Select logging level" />
</SelectTrigger>
<SelectContent>
Expand Down