[UEPR-336] Implement Semantic Layout in the Editor #394
Conversation
kbangelov
force-pushed
the
task/uepr-336-implement-semantic-layout-in-the-editor
branch
from
a5c5012 to
45d5ac7
Compare
kbangelov
force-pushed
the
task/uepr-336-implement-semantic-layout-in-the-editor
branch
from
cc004e0 to
48a10bd
Compare
| onMouseLeave, | ||
| onMore | ||
| onMore, | ||
| ariaRole |
There was a problem hiding this comment.
Nitpick: Let's sort properties alphabetically when possible for readibility
There was a problem hiding this comment.
Yeah, I clearly forgot about it while changing the name
| b) for combined stage menu + stage + sprite/stage selectors | ||
| */ | ||
| flex-direction: row; | ||
| height: 100%; |
There was a problem hiding this comment.
I imagine this was no longer needed because of the body-wrapper css class?
There was a problem hiding this comment.
Yes, that class being on the inside used to say "make up 100% of the body wrapper height"
| /> | ||
| <Tab | ||
| className={tabClassNames.tab} | ||
| aria-label="Code Editor Tab" |
There was a problem hiding this comment.
I rather meant for the aria-labels to be on the components rendering the tabs (so the <Box wrapping the Blocks, the <CostumeTab component, the <SoundTab component). I see that I wasn't very clear what I meant in my previous comment, sorry about that.
In the case of the <Tab components, we shouldn't have an issue there, as the messages describing them (e.g. the FormattedMessage below translates into a Code , should make their purpose clear.
| loading, | ||
| manuallySaveThumbnails, | ||
| onUpdateProjectThumbnail, | ||
| ariaRole, |
There was a problem hiding this comment.
nitpick: see comment above about alphabetical prop ordering
| className={classNames(styles.stageAndTargetWrapper, styles[stageSize])} | ||
| className={styles.targetWrapper} | ||
| role="region" | ||
| aria-label="Target" |
There was a problem hiding this comment.
Nitpick: "Target" -> "Target Pane"
| selectedTabClassName={tabClassNames.tabSelected} | ||
| selectedTabPanelClassName={tabClassNames.tabPanelSelected} | ||
| onSelect={onActivateTab} | ||
| role="region" |
There was a problem hiding this comment.
Having the element directly under the "main" landmark be a separate region is probably redundant.
Instead, I am split between either not having any regions, besides the backpack in it, or:
- the <TabList becomes a region
- the rendered tab content (the <Box wrapping the Blocks, the <CostumeTab component, the <SoundTab component) also become regions - and only the active one will be rendered (and so selectable) at a single time
As I don't have a strong argument against doing it, I think the latter option (defining additional regions) should be the one to go with.
2 participants
Resolves
https://scratchfoundation.atlassian.net/browse/UEPR-336
Proposed Changes
I have added role and aria-label tags for the elements to be considered as landmarks and be navigated through with a screen reader easily. I have separated the page into three parts :
Those three I have separated into the following:
Editor:
Stage and Target:
Issue With Separating Editor into "editor tabs" and "editor panel"
I intended to do that as well. However I found no viable way to do it. TabList and TabPanel are some MIT code imported as a node module. So I can't really edit them to accommodate props "role" and "ariaLabel" as seen in some of the components in the code. I didn't find any way to overwrite the roles these components receive at a later point and wrapping them in a semantic html component breaks the css. Thus there are the following approaches:
Reason for Changes
None of them were focusable so far.
To be tested with a proper screen reader but it seems to work
I have not fully tested whether they can be navigated through quickly as landmarks yet, because the only semi-adequate screen reader I found was an extension which doesn't let me tab from landmark to landmark but reads them out loud when hovered. Also another extension for landmarks does find all 7 described above. So I have tested it to the best of my abilities, still a check from a proper screen will be much appreciated.