Skip to content

Created valid structure for audio menu #16007

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 3 commits into
base: master
Choose a base branch
from

Conversation

raduanastase8x8
Copy link
Contributor

By using menu, group and menuitemradio we have a more clearly and valid structured html.
Previously

@jitsi-jenkins
Copy link

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

@saghul
Copy link
Member

saghul commented Jun 16, 2025

@Calinteodor can you PTAL?

@@ -232,7 +232,7 @@ const ContextMenuItem = ({
<div
aria-controls = { controls }
aria-disabled = { disabled }
aria-label = { accessibilityLabel }
aria-label = { accessibilityLabel || undefined }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to specify undefined?

@@ -135,39 +135,38 @@ const SpeakerEntry = (props: IProps) => {
}

const { children, isSelected, index, length } = props;
const testLabel = 'Test';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think we usually i18n those as well. seems it was not the case for this one though :) Think you can add it to translates?

tabIndex = { -1 }>
<ContextMenuItemGroup>
<ContextMenuItemGroup
aria-labelledby = { microphoneHeaderId }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can name the prop directly ariaLabelledBy, as it does not act as a HTML element attribute on the component, but as a simple prop, and then you can get rid of the aliasing 'aria-labelledby': ariaLabelledBy in the component. Alternatively, you can let it like this and let it propagate through the ...rest props in the component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants