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

Mathquill accessibility improvements #1142

Merged

Conversation

drgrice1
Copy link
Member

This makes changes to the mqeditor.js code to use the changes in openwebwork/mathquill#33.

There are no actual changes needed to use the "mathspeak" addition for screen reader usage. However, there are changes needed for the focus/blur behavior change.

Also, this takes advantage of the fixed "enter" handler.

Note that since the npm package was not published (and shouldn't be until after the MathQuill pull request is merged), it takes a bit of effort to test this. The commands to do this on an Ubuntu system are below. Those commands should generally work on any system though.

First check out this pull request.

Next, it is recommended that you set npm to install global packages in your user's home directory. You can do this by running npm config set prefix="~/.local". This is not required, but if you don't then global packages (and links used here) will be installed to the global location (/usr/lib/node_modules on Ubuntu). Note that if you don't do this then you will need to use sudo to install a global package or link.

For the rest run the following commands (you may need or want to adapt locations in some cases):

cd
git clone -b mathspeak --single-branch https://github.com/drgrice1/mathquill
cd mathquill
npm ci
npm run build
npm link # (add sudo here if you didn't set the prefix above)
cd /opt/webwork/pg/htdocs
npm uninstall @openwebwork/mathquill # get the npm package out of the way
npm link @openwebwork/mathquill
./generate-assets.js

Clean up afterward. From the /opt/webwork/pg/htdocs directory run:

npm unlink @openwebwork/mathquill
npm unlink @openwebwork/mathquill -q # (add sudo here again if you didn't set the prefix)
rm -r ~/mathquill
git checkout package.json package-lock.json
npm ci

Once the MathQuill pull request is merged, I will publish the npm package and update the package.json file in this pull request. So this should not be merged until after that is done.

@somiaj
Copy link
Contributor

somiaj commented Nov 11, 2024

Testing it out, and so far it is working (had a little trouble getting the link to work, for some reason it didn't update the version in package.json, so I had to manually do that to get it to work). Just describing my tests below, haven't noticed any real issues.

I tested that tab works as described, enter works as described, select works with losing focus as described.

I tested the space working as tab, and it allows some movement around the mathquill input, but sometimes just enters in a space, which I found odd, but understandable. If the cursor is in a position in which tab would have moved to the next element, a space is inserted instead.

I don't have a setup to test the actual mathspeak, but so far the changes are working and haven't noticed any real issues with my tests.

Last I am noticed deprecated warnings when I ran npm ci in mathquill/mathspeak. I assume you are aware of this, just mentioning it in case.

@drgrice1
Copy link
Member Author

Testing it out, and so far it is working (had a little trouble getting the link to work, for some reason it didn't update the version in package.json, so I had to manually do that to get it to work). Just describing my tests below, haven't noticed any real issues.

The link command will not add anything to the package.json file as I gave it. It shouldn't need to for testing. The fallback will still serve the file. If you use npm link -S @openwebwork/mathquill then it will add the link to the package.json file. But then the version will be file:full/path/to/@openwebwork/mathquill. So the link will be http://localhost:3005/pg_files/node_modules/@openwebwork/mathquill/dist/mathquill.css?version=file:full/path/to/@openwebwork/mathquill.

I tested the space working as tab, and it allows some movement around the mathquill input, but sometimes just enters in a space, which I found odd, but understandable. If the cursor is in a position in which tab would have moved to the next element, a space is inserted instead.

That is how it is supposed to work and is not a change with this. If you are in a block that is at depth 2 or more (for instance inside a square root, or in the numerator or denominator of a fraction), then space moves to the next block. But if you are in the root block (the only block at depth 1), then space is space. There is also one exception inside a depth 2 or more block. That is if the cursor is after a comma. Then a space is also a space.

Last I am noticed deprecated warnings when I ran npm ci in mathquill/mathspeak. I assume you are aware of this, just mentioning it in case.

Yes. There isn't anything that we can do about those. We have to wait until mocha fixes its issues with using deprecated dependencies. There is an issue on their github project for this. Apparently it is going to require some changes that break compatibility, and so they won't fix it until the next time they have a macro increase in their version.

@drgrice1
Copy link
Member Author

Also note that the deprecated packages are only used for unit tests via mocha. So there is nothing particularly concerning. In fact, all of MathQuill's dependencies are dev dependencies. So the only security vulnerabilities are in development tools, and nothing in the production build.

Copy link
Contributor

@somiaj somiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything worked in my limited testing (I don't have a setup to test the mathspeak part, but everything else worked as described).

@somiaj
Copy link
Contributor

somiaj commented Nov 20, 2024

After the quick comment in the meeting, would there be any disadvantages to just push betaX.Y versions of our npm packages to node vs having to do local linking for testing? Seems we will have to update to non beta releases anyways with the release each year.

@drgrice1
Copy link
Member Author

As I also mentioned there are some disadvantages. Generally, they are minor though, and mostly clerical in nature. It means that a beta version exists that is published from a fork instead of from the main branch. It could mean that a beta version is published that might later need to be reverted to a previous beta version but republished with a new higher beta version. Either that or deleting a beta version, and then needing to wait 24 hours before you can publish a package with that same beta version again.

I will publish this, and add it here, so that it is simpler for others to test.

Although, if someone wants to help with development of MathQuill and test with webwork2 you might note the process, as that is a good way to do this. If someone wants to help with development of the CodeMirror 6 PG problem editor as well, you actually need to use approach with the pg-codemirror-editor and the codemirror-lang-pg packages just to test the codemirror-lang-pg packages with the pg-codemirror-editor test page (or yet another layer of links to test directly with webwork2).

@drgrice1 drgrice1 force-pushed the mathquill-accessibility-improvements branch from a665721 to 62befa6 Compare November 20, 2024 18:22
@drgrice1
Copy link
Member Author

The npm package is now published and installed in this pull request. So now all you need to do is run npm ci to test this.

@drgrice1 drgrice1 force-pushed the mathquill-accessibility-improvements branch 2 times, most recently from 2f56176 to 752413b Compare November 29, 2024 13:18
@drgrice1 drgrice1 marked this pull request as ready for review December 3, 2024 20:30
@drgrice1 drgrice1 force-pushed the mathquill-accessibility-improvements branch from 752413b to 974503c Compare December 3, 2024 21:22
The "enter" handler is fixed now.  It has never worked before.  This
allows the mqeditor to do what it did before without needed yet another
event listener.
@drgrice1 drgrice1 force-pushed the mathquill-accessibility-improvements branch from 974503c to 2211244 Compare December 11, 2024 02:00
@Alex-Jordan
Copy link
Contributor

I set out to test this. I got as far as npm run build in the instructions, and then I have an error:

alex.jordan@vmwebworkdevw02:~/mathquill $ npm run build

> @openwebwork/mathquill@0.11.0-beta.2 build
> npm run build:web && npm run build:lib


> @openwebwork/mathquill@0.11.0-beta.2 build:web
> webpack --mode production

Using production mode.
assets by status 1.99 MiB [cached] 7 assets
Entrypoint mathquill = mathquill.css mathquill.js
orphan modules 307 KiB (javascript) 1.87 MiB (asset) 1.06 KiB (runtime) [orphan] 48 modules
cacheable modules 291 KiB (javascript) 11.4 KiB (css/mini-extract)
  ./src/index.ts + 32 modules 291 KiB [built] [code generated]
  css ./node_modules/css-loader/dist/cjs.js!./node_modules/less-loader/dist/cjs.js??ruleSet[1].rules[2].use[2]!./src/css/main.less 11.4 KiB [built] [code generated]

ERROR in [eslint] Config (unnamed): Key "rules": Key "constructor-super": structuredClone is not defined

webpack 5.94.0 compiled with 1 error in 9157 ms

Do you know what this might be about?

@Alex-Jordan
Copy link
Contributor

Actually before that, I have lots of npm warnings. Like this first one (followed by dozens more like it):

alex.jordan@vmwebworkdevw02:~/mathquill $ npm ci
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@awmottaz/prettier-plugin-void-html@1.6.1',
npm WARN EBADENGINE   required: { node: '>=18.0.0' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }

I'm not sure what this means.

@Alex-Jordan
Copy link
Contributor

Er, I guess it means I have an old version of node or npm that I need to upgrade. I will look into that now.

@somiaj
Copy link
Contributor

somiaj commented Dec 11, 2024

Are you trying to follow the original instructions? They shouldn't be needed now, just clone the branch and build like normal with npm ci.

@Alex-Jordan
Copy link
Contributor

Oh, I see that now at the end. FWIW, I upgraded node to the latest 22, and had even more errors. Then I installed version 18, and I was making progress.

@Alex-Jordan
Copy link
Contributor

So, I'm confused then because the mathquill PR is still open: openwebwork/mathquill#33

For testing this now, is there anything more to do that checking out this branch, running npm ci, and restarting webwork2?

@somiaj
Copy link
Contributor

somiaj commented Dec 11, 2024

My understanding is there shouldn't be. A beta version was published and this will download that version to test.

@somiaj
Copy link
Contributor

somiaj commented Dec 11, 2024

You might have to undo any local link you were trying to setup depending on how far you got.

@Alex-Jordan
Copy link
Contributor

You might have to undo any local link you were trying to setup depending on how far you got.

I did all the cleanup steps in the original instructions.

OK, I got to testing. I can test with Apple VoiceOver. When I enter a math input, I typed x^ and I hear "x superscript". So far so good. Then if I backspace, the cursor leaves superscript and I just see the "x" with cursor on the base line after the "x". And I hear "superscript", which seems odd. If I backspace again, removing the "x", I hear "x". Is it expected that when you backspace, you hear what is removed?

When focus is in the input and I hit tab, it is still cycles me through the toolbar buttons. So I am wondering if I have not done something right. I have this pg branch, I ran npm ci fresh in pg/htdocs (as well as in webwork2/htdocs), and I restarted webwork2. Or maybe I am thinking of a behavior that we discussed, but is not part of this PR? I am thinking something like tab was going to bring focus into the toolbar, and then tabbing again would go past the toolbar. Arrow keys would let you go to buttons within the toolbar. Is that something that was just discussed and is not part of this?

@Alex-Jordan
Copy link
Contributor

I convinced myself the other things I mentioned about tabbing are just from a discussion, not part of this PR. I tested all the things mentioned in the mathquill PR and it all works. I'm going to merge this and also that PR. That PR has no approvals but I'm assuming it's the right thing to do if this one is merged.

@Alex-Jordan Alex-Jordan merged commit d0ea4b1 into openwebwork:develop Dec 11, 2024
3 checks passed
@drgrice1
Copy link
Member Author

OK, I got to testing. I can test with Apple VoiceOver. When I enter a math input, I typed x^ and I hear "x superscript". So far so good. Then if I backspace, the cursor leaves superscript and I just see the "x" with cursor on the base line after the "x". And I hear "superscript", which seems odd. If I backspace again, removing the "x", I hear "x". Is it expected that when you backspace, you hear what is removed?

That is the expected behavior in terms of how this is implemented. I think the idea is that it voices what you have just deleted, and not what is now showing. I don't know that it is correct, but that is how the Desmos code is set up to work. I think the Desmos implementation is still a work in progress on this also. I was watching their changes as I worked on this, and saw several changes to their code as I did so. I pulled in their changes, when I saw them. I will continue to monitor what they do and integrate their changes into our code, but we can also change things as needed. This is just a start, and I am sure lots more work will be needed.

When focus is in the input and I hit tab, it is still cycles me through the toolbar buttons. So I am wondering if I have not done something right. I have this pg branch, I ran npm ci fresh in pg/htdocs (as well as in webwork2/htdocs), and I restarted webwork2. Or maybe I am thinking of a behavior that we discussed, but is not part of this PR? I am thinking something like tab was going to bring focus into the toolbar, and then tabbing again would go past the toolbar. Arrow keys would let you go to buttons within the toolbar. Is that something that was just discussed and is not part of this?

I did not implement the arrow key cycling through the buttons. You told me not to put effort into that until you looked into whether or not that is the right thing to do. I did notice that with Gnome Orca when focus moves to the toolbar when you type tab from the input, that it switches to "browse mode", and then the arrow keys actually already work to move between buttons in the toolbar.

@drgrice1 drgrice1 deleted the mathquill-accessibility-improvements branch December 11, 2024 13:19
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