Skip to content

improvement: add aria-label(ledby) for lists #5030

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
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
130 changes: 69 additions & 61 deletions packages/frontend/src/components/AccountListSidebar/AccountItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -242,70 +242,78 @@ export default function AccountItem({
// for a different account, and upon initial render.

return (
<button
className={classNames(styles.account, rovingTabindex.className, {
[styles.active]: isSelected,
[styles['context-menu-active']]: isContextMenuActive,
<li
className={classNames(styles.accountWrapper, {
[styles.isSticky]: isSticky,
'unconfigured-account': account?.kind !== 'Configured',
})}
// TODO consider adding `role='tabpanel'` for the main area of the app.
// Although screen readers might start to announce
// the account name every time you focus something in the main area,
// which might be too verbose.
role='tab'
aria-selected={isSelected}
aria-busy={!account}
onClick={() => onSelectAccount(accountId)}
onContextMenu={onContextMenu}
onMouseEnter={() => account && updateAccountForHoverInfo(account, true)}
onMouseLeave={() => account && updateAccountForHoverInfo(account, false)}
x-account-sidebar-account-id={accountId}
data-testid={`account-item-${accountId}`}
ref={ref}
tabIndex={rovingTabindex.tabIndex}
onFocus={rovingTabindex.setAsActiveElement}
onKeyDown={rovingTabindex.onKeydown}
>
{!account ? (
<div className={styles.avatar}>
<div className={styles.content}>⏳</div>
</div>
) : account.kind == 'Configured' ? (
<div className={styles.avatar}>
{' '}
{account.profileImage ? (
<img
className={styles.content}
src={runtime.transformBlobURL(account.profileImage)}
/>
) : (
<div
className={styles.content}
style={{ backgroundColor: account.color }}
>
{avatarInitial(
account.displayName || '',
account.addr || undefined
)}
</div>
)}
</div>
) : (
<div className={styles.avatar}>
<div className={styles.content}>?</div>
</div>
)}
{muted && (
<div
aria-label='Account notifications muted'
className={styles.accountMutedIconShadow}
>
<Icon className={styles.accountMutedIcon} icon='audio-muted' />
</div>
)}
<div className={classNames(styles.accountBadge)}>{badgeContent}</div>
</button>
<button
className={classNames(styles.account, rovingTabindex.className, {
[styles.active]: isSelected,
[styles['context-menu-active']]: isContextMenuActive,
[styles.isSticky]: isSticky,
'unconfigured-account': account?.kind !== 'Configured',
})}
// TODO consider adding `role='tabpanel'` for the main area of the app.
// Although screen readers might start to announce
// the account name every time you focus something in the main area,
// which might be too verbose.
role='tab'
aria-selected={isSelected}
aria-busy={!account}
onClick={() => onSelectAccount(accountId)}
onContextMenu={onContextMenu}
onMouseEnter={() => account && updateAccountForHoverInfo(account, true)}
onMouseLeave={() =>
account && updateAccountForHoverInfo(account, false)
}
x-account-sidebar-account-id={accountId}
data-testid={`account-item-${accountId}`}
ref={ref}
tabIndex={rovingTabindex.tabIndex}
onFocus={rovingTabindex.setAsActiveElement}
onKeyDown={rovingTabindex.onKeydown}
>
{!account ? (
<div className={styles.avatar}>
<div className={styles.content}>⏳</div>
</div>
) : account.kind == 'Configured' ? (
<div className={styles.avatar}>
{' '}
{account.profileImage ? (
<img
className={styles.content}
src={runtime.transformBlobURL(account.profileImage)}
/>
) : (
<div
className={styles.content}
style={{ backgroundColor: account.color }}
>
{avatarInitial(
account.displayName || '',
account.addr || undefined
)}
</div>
)}
</div>
) : (
<div className={styles.avatar}>
<div className={styles.content}>?</div>
</div>
)}
{muted && (
<div
aria-label='Account notifications muted'
className={styles.accountMutedIconShadow}
>
<Icon className={styles.accountMutedIcon} icon='audio-muted' />
</div>
)}
<div className={classNames(styles.accountBadge)}>{badgeContent}</div>
</button>
</li>
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default function AccountListSidebar({
}: Props) {
const tx = useTranslationFunction()

const accountsListRef = useRef<HTMLDivElement>(null)
const accountsListRef = useRef<HTMLUListElement>(null)
const { openDialog } = useDialog()
const [accounts, setAccounts] = useState<number[]>([])
const [{ accounts: noficationSettings }] = useAccountNotificationStore()
Expand Down Expand Up @@ -144,8 +144,12 @@ export default function AccountListSidebar({
data-tauri-drag-region
/>
)}
<div
<ul
ref={accountsListRef}
// Perhaps just "Profiles" would be more appropriate,
// because you can do other things with profiles in this list,
// but we have the same on Android.
aria-label={tx('switch_account')}
className={styles.accountList}
onScroll={updateHoverInfoPosition}
role='tablist'
Expand All @@ -164,9 +168,11 @@ export default function AccountListSidebar({
muted={noficationSettings[id]?.muted || false}
/>
))}
<AddAccountButton onClick={onAddAccount} />
<li>
<AddAccountButton onClick={onAddAccount} />
</li>
</RovingTabindexProvider>
</div>
</ul>
{/* The condition is the same as in https://github.com/deltachat/deltachat-desktop/blob/63af023437ff1828a27de2da37bf94ab180ec528/src/renderer/contexts/KeybindingsContext.tsx#L26 */}
{window.__screen === Screens.Main && (
<div className={styles.buttonsContainer}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@
--als-avatar-margin: 5px;
--als-active-indicator-color: white;

margin: 0;
padding: 0;
list-style: none;

align-items: center;
display: flex;
flex-direction: column;
Expand All @@ -63,6 +67,28 @@
}
}

.accountWrapper {
margin-bottom: var(--als-avatar-margin);
margin-top: var(--als-avatar-margin);

&.isSticky {
position: sticky;
bottom: var(--als-avatar-margin);
top: var(--als-avatar-margin);
// Only needed when this account is scrolled _above_ the visible region.
z-index: map-get($z-index, account-list-sidebar-scope-sticky-account);

.account {
.avatar {
opacity: 1;
.content {
box-shadow: 0px 0px 4px 2px #00000040;
}
}
}
}
}

.account {
border: none;
background: none;
Expand All @@ -71,8 +97,6 @@

height: var(--als-avatar-size);

margin-bottom: var(--als-avatar-margin);
margin-top: var(--als-avatar-margin);
// Adding extra `var(--als-avatar-size) + 2 * var(--als-avatar-margin)`
// to the margins so that
// if there is another `.isSticky` account, then that account does not
Expand Down Expand Up @@ -107,21 +131,6 @@
}
}

&.isSticky {
position: sticky;
bottom: var(--als-avatar-margin);
top: var(--als-avatar-margin);
// Only needed when this account is scrolled _above_ the visible region.
z-index: map-get($z-index, account-list-sidebar-scope-sticky-account);

.avatar {
opacity: 1;
.content {
box-shadow: 0px 0px 4px 2px #00000040;
}
}
}

&.active,
&:hover,
&.context-menu-active {
Expand Down
Loading
Loading