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

fix(poll): structure #15645

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

joshuai96
Copy link
Contributor

@joshuai96 joshuai96 commented Feb 20, 2025

When using a screen reader the polls are not easily perceivable.

This is related to WCAG Success Criterion 1.3.1 Info and Relationships.

  • Checkbox changes:
    The checkbox component now uses a given id, the name or a random id to link it's input to the label. This further optimizes the perception with assistive technologies.
  • Packages update:
    The type packages missed the last update and using the useId hook failed linting.
  • Poll:
    The question and answers were not grouped together logically enough for assistive technologies like a screen reader. Wrapping in a fieldset with a legend binds them together better instead of the ul element. The poll result is logically still an unordered list. The bind the labels and checkboxes together the pollId and index of the answers is used as id.
  • PollsList:
    The message There are no polls in the meeting yet. Start a poll here! was wrapped in a <span>. According to the WCAG compliance test, a <span> must only be used inside of <p> (paragraph) to mark specific text passages. A single <span> element as simple text wrapper is discouraged.

@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 :(.

@joshuai96
Copy link
Contributor Author

Drafting to address unsuccessful checks

@joshuai96 joshuai96 marked this pull request as draft February 20, 2025 15:30
@joshuai96 joshuai96 marked this pull request as ready for review February 24, 2025 13:33
@joshuai96 joshuai96 marked this pull request as draft February 24, 2025 13:37
@joshuai96
Copy link
Contributor Author

Hello @saghul or who it may concern
as mentioned in this PR, I've updated the react types dev dependencies as they seem to be skipped in #13241.
This breakes the CI, I think a lot of the codebase is basically not ready to run on react v18, as indicated by the warning in the console, when starting the dev server:

Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https://reactjs.org/link/switch-to-createroot

Strangely running in v17 types, the used useId hook is not known in the types, but exists as long as v16 🤷‍♂️.

Should I open an issue about the v17 to v18 discrepancy? Currently I can not allocate time to handle this myself, but this PR is blocked by it.

Signed-off-by: Joshua Irmer <[email protected]>
@saghul
Copy link
Member

saghul commented Mar 4, 2025

Should I open an issue about the v17 to v18 discrepancy? Currently I can not allocate time to handle this myself, but this PR is blocked by it.

Sure, feel free to do so!

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.

3 participants