-
Notifications
You must be signed in to change notification settings - Fork 990
style(desktop): unified settings vocabulary across every page #4035
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
f11bd59
2482984
ec0c665
4c35764
4c0fe8e
ccc7a06
54f5729
9a676d9
fd03a31
87ffc80
9495ba8
6ebcf88
f8c933c
a4fc71e
d173004
89c764c
8d3bf74
44defaa
c0db7b7
8e0f9ea
2f652a1
848a3a5
e1be06e
7647bf5
f4c810c
bfe32fb
835f177
4bd4e9b
157c94b
4cfb272
1e447a5
2df2c8d
b096442
9421c53
6b64621
4343edc
4ae0b10
6eb4a13
602ec3e
fa6722e
d227690
ea57854
c538f08
6294dfd
3533b90
5fe928e
3052ad8
368c986
7542529
4243723
38f9104
0de61e5
5b786f8
78d3f1c
9ddc44a
68cdb21
1688dca
9d6a23a
ae993d9
a0bde86
94d5c95
087dc9b
6033eff
0b236b0
6c376f0
1fcc0c3
6b07576
ae3ef0e
a8c9bb4
1bc1780
3d552d3
e2b2b0d
cc32565
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,4 +1,6 @@ | ||
| export { | ||
| readLastMigrationRunAt, | ||
| useMigrateV1DataToV2, | ||
| V1_MIGRATION_LAST_RUN_AT_EVENT, | ||
| V1_MIGRATION_SUMMARY_EVENT, | ||
| } from "./useMigrateV1DataToV2"; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,28 +1,26 @@ | ||||||||||||||||
| import { Card, CardContent } from "@superset/ui/card"; | ||||||||||||||||
| import { Skeleton } from "@superset/ui/skeleton"; | ||||||||||||||||
|
|
||||||||||||||||
| export function ProfileSkeleton() { | ||||||||||||||||
| return ( | ||||||||||||||||
| <Card> | ||||||||||||||||
| <CardContent> | ||||||||||||||||
| <ul className="space-y-6"> | ||||||||||||||||
| <li className="flex items-center justify-between gap-8 pb-6 border-b border-border"> | ||||||||||||||||
| <div className="flex-1"> | ||||||||||||||||
| <Skeleton className="h-4 w-16 mb-2" /> | ||||||||||||||||
| <Skeleton className="h-3 w-40" /> | ||||||||||||||||
| </div> | ||||||||||||||||
| <Skeleton className="h-8 w-8 rounded-full" /> | ||||||||||||||||
| </li> | ||||||||||||||||
| <li className="flex items-center justify-between gap-8 pb-6 border-b border-border"> | ||||||||||||||||
| <Skeleton className="h-4 w-12" /> | ||||||||||||||||
| <Skeleton className="h-10 flex-1" /> | ||||||||||||||||
| </li> | ||||||||||||||||
| <li className="flex items-center justify-between gap-8"> | ||||||||||||||||
| <Skeleton className="h-4 w-12" /> | ||||||||||||||||
| <Skeleton className="h-10 flex-1" /> | ||||||||||||||||
| </li> | ||||||||||||||||
| </ul> | ||||||||||||||||
| </CardContent> | ||||||||||||||||
| </Card> | ||||||||||||||||
| <div className="space-y-3"> | ||||||||||||||||
| <div className="flex items-center justify-between gap-8"> | ||||||||||||||||
| <div className="flex-1"> | ||||||||||||||||
| <Skeleton className="h-4 w-16 mb-2" /> | ||||||||||||||||
| <Skeleton className="h-3 w-40" /> | ||||||||||||||||
| </div> | ||||||||||||||||
| <div className="flex items-center gap-3"> | ||||||||||||||||
| <Skeleton className="size-12 rounded-full" /> | ||||||||||||||||
| <Skeleton className="h-8 w-20" /> | ||||||||||||||||
| </div> | ||||||||||||||||
|
Comment on lines
+11
to
+14
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. Skeleton has a phantom rectangular placeholder that doesn't correspond to the rendered Avatar row. The actual Avatar 🔧 Proposed fix <div className="flex items-center gap-3">
<Skeleton className="size-12 rounded-full" />
- <Skeleton className="h-8 w-20" />
</div>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| </div> | ||||||||||||||||
| <div className="flex items-center justify-between gap-8"> | ||||||||||||||||
| <Skeleton className="h-4 w-12" /> | ||||||||||||||||
| <Skeleton className="h-9 w-80" /> | ||||||||||||||||
| </div> | ||||||||||||||||
| <div className="flex items-center justify-between gap-8"> | ||||||||||||||||
| <Skeleton className="h-4 w-12" /> | ||||||||||||||||
| <Skeleton className="h-9 w-80" /> | ||||||||||||||||
| </div> | ||||||||||||||||
| </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.
Email input has no accessible name — WCAG 1.3.1 Level A gap.
The
SettingRowhelper uses a plain<div>for the label with nohtmlForprop, and theInputelements have noidto form a programmatic association. The Email field is the most critical — it has neither a placeholder nor anaria-label, so assistive technologies have nothing to announce as the control's label. The Name input is partially covered by itsplaceholder, but a real<label>association is still required per WCAG.OrganizationSettings.tsx'sSettingsRowalready solves this correctly with<Label htmlFor={htmlFor}>+ matchingidon eachInput. AligningSettingRowhere to the same pattern fixes both the accessibility gap and the inconsistency.🛡️ Proposed fix
Then add
id/htmlForat each call-site:🤖 Prompt for AI Agents