Test for EditorAccessibility

[ofhope]

Changes:

After creating a story for this component it became apparent that the visual story was redundant. A better way to document this components behaviour is through unit tests.

#2398

• Updated the prop types to oneOf to limit down the severity options. This matches the severity options in CodeMirror where these messages are generated from. https://codemirror.net/docs/ref/#lint.Diagnostic.severity
• Removed tailing white space from translations
• Test for empty messages
• Test for single message

---

I think this component is malformed. It looks like some work has gone into enhancing aria properties for the line number. But it contains an empty string. Perhaps it was intended to be a sub component?

<span
  className="editor-linenumber"
  aria-live="polite"
  aria-atomic="true"
  id="current-line"
>
    {' '}

It appears redundant to me. I'd be in favour of removing it for now until its known what it is used for.

---

An interesting post on inclusively hiding content. I was wondering if its worth updating %hidden-element to use clip, rather than positioning 10000px off screen. They both seem to remain available to assistive technology (I haven't verifyied). But clipping isn't relying on a large offset to hide.
https://www.scottohara.me/blog/2017/04/14/inclusively-hidden.html

I have verified that this pull request:

• has no linting errors (npm run lint)
• has no test errors (npm run test)
• is from a uniquely-named feature branch and is up to date with the develop branch.
• is descriptively named and links to an issue number, i.e. Fixes #123

[lindapaiste]

I think this component is malformed. It looks like some work has gone into enhancing aria properties for the line number. But it contains an empty string. Perhaps it was intended to be a sub component?

This component is definitely not the best so feel free to rework it! The text of this span is actually getting set by the Editor component using direct DOM manipulation 😱

p5.js-web-editor/client/modules/IDE/components/Editor/index.jsx

Lines 198 to 201 in d5eee45

	const temp = this.props.t('Editor.KeyUpLineNumber', {
	lineNumber: parseInt(this._cm.getCursor().line + 1, 10)
	});
	document.getElementById('current-line').innerHTML = temp;

(Introduced in this commit db85dcc)

The lineNumber should definitely be a prop. I opened an issue #2174 about this but I haven't fixed it.

The line number should be stored as a state and then passed down as a prop to the EditorAccessibility component, which contains the 'current-line' element.

[ofhope]

Oh ouch. OK, I think storing the line number to local state and passing it down as a prop would work. But this would trigger the whole component to re-render on each keyup event as the new line value was set. We might want to expose the code mirror instance to EditorAccessibility so it can attach it's own state management... I'll think on this more and have a crack at updating.

Side note. I'm not sure why we're only recomputing line number on the keyup event. I'd expect a mouse interaction would also change the text cursor and not trigger keyup. It looks like CodeMirror has another callback option available specifically for cursor this._cm.on('cursorActivity')

[ofhope]

I'm still intending on looking into this. Work deliverables have been taking priority.

I think extracting the Code Mirror instance out to a React Context would work to expose access to all child components. Generally I'd prefer to avoid context if possible as they add a little magic to the code as to where an instance was initialised.

[lindapaiste]

Oh ouch. OK, I think storing the line number to local state and passing it down as a prop would work. But this would trigger the whole component to re-render on each keyup event as the new line value was set.

I think it would only cause a re-render if the lineNumber is different?

Side note. I'm not sure why we're only recomputing line number on the keyup event. I'd expect a mouse interaction would also change the text cursor and not trigger keyup. It looks like CodeMirror has another callback option available specifically for cursor this._cm.on('cursorActivity')

It's probably safe to assume that whoever wrote it didn't know about the cursorActivity event, and used whatever they could to get it working.

[lindapaiste]

We've had a lot of discussion here about future improvements but I'm going to go ahead and merge in the these tests for now. Thanks!