You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/08/20 11:24:03 UTC
Re: [PR] chore(ui): show signed-in mark in model provider list [maka]
Astro-Han commented on code in PR #3315:
URL: https://github.com/apache/maka/pull/3315#discussion_r3821068237
##########
apps/desktop/src/renderer/settings/provider-catalog-page.tsx:
##########
@@ -144,7 +144,17 @@ export function ProviderCatalogPage(props: {
startContent={<ProviderLogo type={card.providerType} />}
label={/* a11y-allow: this label names the ROW, not the span. Astryx's Item puts consumer props on its outer wrapper and renders a separate invisible <button> for the click target, so an aria-label on the Item never reaches that button — measured. The button is named from its content, and this span is how the status reaches that name. Removing it drops the runtime error from the row's accessible name (settings.spec:226).*/ <span aria-label={providerCopy.oauthSection.cardAria(card.name, card.status, card.description)}>{card.name}</span>}
description={card.description}
- endContent={<ChevronRight size={ICON_SIZE.chrome} aria-hidden="true" />}
+ endContent={(
+ <HStack gap={2} vAlign="center">
+ {card.isLoggedIn && (
+ <span className="settingsStatus" aria-hidden="true">
+ <Check size={ICON_SIZE.chrome} />
Review Comment:
Test gap rather than a defect, so no P level. Nothing in this PR is covered by a test, and the row this touches already has desktop e2e coverage — the a11y comment two lines up cites `settings.spec:226` for the accessible name of exactly this `ListItem`. An assertion in that spec that a signed-in card's row renders the mark, and a signed-out one does not, would be a few lines and would pin the one thing a future refactor of `endContent` could silently drop. Not blocking from my side, but the PR template's "Tests cover the change and fail without it" is currently unmet.
##########
apps/desktop/src/renderer/settings/provider-catalog-page.tsx:
##########
@@ -144,7 +144,17 @@ export function ProviderCatalogPage(props: {
startContent={<ProviderLogo type={card.providerType} />}
label={/* a11y-allow: this label names the ROW, not the span. Astryx's Item puts consumer props on its outer wrapper and renders a separate invisible <button> for the click target, so an aria-label on the Item never reaches that button — measured. The button is named from its content, and this span is how the status reaches that name. Removing it drops the runtime error from the row's accessible name (settings.spec:226).*/ <span aria-label={providerCopy.oauthSection.cardAria(card.name, card.status, card.description)}>{card.name}</span>}
description={card.description}
- endContent={<ChevronRight size={ICON_SIZE.chrome} aria-hidden="true" />}
+ endContent={(
+ <HStack gap={2} vAlign="center">
+ {card.isLoggedIn && (
Review Comment:
Contract observation, not a defect, so no P level. `isLoggedIn` in `provider-oauth-section.tsx:148-152` is true for `authenticated`, `refreshing`, `quota_unavailable` **and** `provider_rejected`, so a provider whose credentials the provider itself has rejected now carries an affirmative check mark reading "Signed in". That classification predates this PR and already fed `card.status` and the row's accessible name, so the change is consistent rather than newly wrong — but a check mark is a stronger claim than an aria string, and `provider_rejected` is precisely the state a user would open this list to notice. Worth deciding deliberately whether the mark should track `authenticated | refreshing` only and leave the broken states to the description. Confirmed by reading the source at this head.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org