microsoft / FluidFramework

Library for building distributed, real-time collaborative web applications
https://fluidframework.com
MIT License
4.74k stars 532 forks source link

Dice roller example is not accessible with screenreader #3620

Closed bmeersma closed 3 years ago

bmeersma commented 4 years ago

The dice roller example on https://fluidframework.com/playground/?path=/docs/react-demos-dice-roller--demo is not accessible with a screenreader. Two improvements that could be made.

  1. Use an aria-alert to announce the new dice value after each role
  2. Set the aria-label as the value of the dice
atiqueahmedziad commented 4 years ago

can I work on it?

tylerbutler commented 4 years ago

can I work on it?

@atiqueahmedziad Sure! I think the changes should be made in the examples/data-objects/dice-roller project. Thank you! I have assigned it to you. Please mention me here if you have any questions!

@SamBroner Are there additional changes needed in https://github.com/microsoft/FluidStorybook other than updating the dependency once the fix is committed here?

atiqueahmedziad commented 4 years ago

The dice roller example on https://fluidframework.com/playground/?path=/docs/react-demos-dice-roller--demo is not accessible with a screenreader. Two improvements that could be made.

1. Use an aria-alert to announce the new dice value after each role

2. Set the aria-label as the value of the dice

@tylerbutler Do we need both aria-label and aria-alert for this regard? If So, I had this following solution in mind.

 const DiceRollerView: React.FC<IDiceRollerViewProps> = (props: IDiceRollerViewProps) => {
+    const [diceRolledAlert, setDiceRolledAlert] = React.useState("");
     const [diceValue, setDiceValue] = React.useState(props.model.value);

     React.useEffect(() => {
         const onDiceRolled = () => {
             setDiceValue(props.model.value);
+            setDiceRolledAlert("alert");
         };
         props.model.on("diceRolled", onDiceRolled);
         return () => {
@@ -59,7 +61,7 @@ const DiceRollerView: React.FC<IDiceRollerViewProps> = (props: IDiceRollerViewPr

     return (
         <div>
-            <span style={{ fontSize: 50 }}>{diceChar}</span>
+            <span role={diceRolledAlert} aria-label={diceValue.toString()} style={{ fontSize: 50 }}>{diceChar}</span>
             <button onClick={props.model.roll}>Roll</button>
         </div>
     );
tylerbutler commented 4 years ago

@bmeersma Can you comment on the questions @atiqueahmedziad raises above?

bmeersma commented 4 years ago

@tylerbutler @atiqueahmedziad yes, I think we should have both a label and an alert. The alert will allow users to spin the dice and learn the value without navigating back to the dice and the label will allow users to learn that there are dice and the value of the dice while navigating through the webpage. Thanks for the questions.

micahgodbolt commented 4 years ago

I've gone in and done most of what @atiqueahmedziad has suggested (with a little extra do deal with the fact that two demos change on each click). PR is up https://github.com/microsoft/FluidStorybook/pull/66.

micahgodbolt commented 3 years ago

This issue got resolved via this PR https://github.com/microsoft/FluidStorybook/pull/66