Improve Reviews by Product accessibility

[Aljullu]

Part of #658.

This PR improves the Reviews by Product block accessibility so it can be easily used with a screen reader. It implements the suggestion from #841 (comment) and adds some notices when the reviews list is updated.

This PR adds a new dependency to the frontend @wordpress/a11y. It's quite lightweight and wasn't easy to replace with vanilla JS.

Accessibility

• I've tested using only a keyboard (no mouse)
• I've tested using a screen reader
• All animations respect prefers-reduced-motion
• All text has at least a 4.5 color contrast with its background

How to test the changes in this Pull Request:

1. Create a post with a Reviews by Product block.
2. Using a screen reader, navigate the post.
3. Notice it reads Order reviews by (instead of Order by) and Load more reviews (intead of Load more), making it easy to know what the select and the button refer too.
4. Update the order and click on Load more and verify it announces the list has been updated when the new reviews have been loaded.

[nerrad]

Code wise looks pretty good, however I am getting this error in the editor for the Load more button:

Warning: Failed prop type: The prop `onClick` is marked as required in `LoadMoreButton`, but its value is `undefined`.

The error in my case appeared when I started with content that was not restricted by the load more boundaries and then decreased the Starting number of reviews to a number less than the total reviews available.

And the load more button is not only non-functional in the editor but is also styled natively:

However, on the frontend, the load more button is both functional and styled correctly.

[Aljullu]

Code wise looks pretty good, however I am getting this error in the editor for the Load more button:

My bad. 🤦‍♂️ Should be fixed in a0aa5ae.

non-functional

I guess that's to be expected in the editor, right? Making blocks interactive in the editor might be confusing for users because when publishing the post, they will always get the initial state defined by attributes. So I think the editor must always display that initial state instead of reacting to controls.

but is also styled natively

Shouldn't that be implemented by themes? We don't know how each theme styles buttons, so adding an specific style to the editor would generate unexpected results.

[nerrad]

I guess that's to be expected in the editor, right? Making blocks interactive in the editor might be confusing for users because when publishing the post, they will always get the initial state defined by attributes. So I think the editor must always display that initial state instead of reacting to controls.

Personally I think it'll be equally confusing for users who go to click the button and nothing happens. I understand the reasoning for scenarios where having blocks interactive in the editor could add more complexity for users but I don't think this is one of them. Load more... imo is similar to the "read more..." functionality here. At a minimum, if we're not going to make this action functional in the editor we should provide some indication to the user for why it is disabled. Accessibility likely applies here as well.

Shouldn't that be implemented by themes? We don't know how each theme styles buttons, so adding an specific style to the editor would generate unexpected results.

Sure, I'm using storefront, shouldn't the style of the button in the editor match the frontend then? Is that something Storefront will need to account for? I know this potentially strays into a general GB problem (themes have to support the editor for styles), but I think at a minimum we should make note of any things that our own theme can be serve as an example for?

[Aljullu]

Personally I think it'll be equally confusing for users who go to click the button and nothing happens. I understand the reasoning for scenarios where having blocks interactive in the editor could add more complexity for users but I don't think this is one of them. Load more... imo is similar to the "read more..." functionality here. At a minimum, if we're not going to make this action functional in the editor we should provide some indication to the user for why it is disabled.

Besides the Load more button, there is also the Order by select. What I'm concerned about it is that if we let users interact with the Order by select in the editor, should it update the attributes or not? A user might expect that after changing the select value in the editor, attributes will be updated, since that's what they are seeing in the screen, right?

But then, why not doing the same with Load more? If a user clicks on Load more so 20 reviews become visible, should that update the Starting Number of Reviews attribute or not? Wouldn't it be confusing that changes in the Order by select update the attributes while clicking on Load more doesn't?

I don't have any strong opinion here and I agree with your points, but at the same time, buttons from product grids like Products by Category or the Button block itself don't trigger the same action in the editor and the frontend, so my thought is that users already know buttons and other interactive elements behave differently in the editor and the frontend.

Regarding why Read more (to expand one review's text) is enabled in the editor while Load more (to load more reviews) is not I don't have an answer for that. But my point of view is that Read more can be undone (you have a Hide full review button), while the only way to recover the initial status after Load more is clicked would be reloading the page.

Accessibility likely applies here as well.

Agree, we might want to wrap the block with <Disabled> in the editor? I pushed a commit that does that: 2fdaa59. I'm happy to revert it if we decide otherwise, I just pushed the commit so it's easier to test it. 🙂

Sure, I'm using storefront, shouldn't the style of the button in the editor match the frontend then? Is that something Storefront will need to account for? I know this potentially strays into a general GB problem (themes have to support the editor for styles), but I think at a minimum we should make note of any things that our own theme can be serve as an example for?

Right, those are good points. Another alternative would be copying the classes from the button block, like it looks like the product grids do. I created a testing PR that does that: #871.

If it sounds good let's move the discussion about styles there so we don't keep blocking this PR.

[nerrad]

If it sounds good let's move the discussion about styles there so we don't keep blocking this PR.

Sounds good 👍