-
Notifications
You must be signed in to change notification settings - Fork 209
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
include a11y in default settings, and fix a number of problems with changing menu values. (mathjax/MathJax#3310) #1169
base: develop
Are you sure you want to change the base?
Conversation
…hanging menu values. (mathjax/MathJax#3310)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question, we might want to discuss before merging.
Btw., we can also make a new version of mj-context-menu
if we need to.
// | ||
const store = this.store; | ||
const active = store.active; | ||
(store as any)._active = document.body; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we use store.active = document.body
here? I assume it is not one of the HTML elements the menu is aware of?
So if I understand correctly, we ask super
to set focus on the body. Does that mean that tabbing would start at the beginning of the page again? Is that what we want?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, tabbing would restart from the top. I hadn't thought of that. What do you suggest? We should chat about this in our next meeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant to comment not approve.
This PR fixes a number of problems that occur when menu values are changed, and in particular, when the menu values are reset to their defaults.
The fixes include the following:
Resetting the menu to default values should force the math to be retypeset, but instead caused the math to disappear. This was due to the initial typesetting not setting the
MathJax.startup.rerenderPromise
, so that has been added, here.Turning off inline breaking would not remove the current inline breaks. This was due to the fact the inline breaks are handled via properties of the internal MathML nodes, and they were not being cleared. This PR adds code to clear those properties. It also moves the marker that the math has inline breaks from the
outputData
object (which is cleared when the state is set to before the math is rendered, so the flag was being lost) to a property of the top-level math node.In the past, a menu that is produced by a mouse click (rather than pressing space on a focused expression) could cause the expression to become focused even when it has been re-rendered, which could leave the expression in a partially active state. We add a
refocus
property to theMJContextMenu
class to track whether we should refocus the expression or not. This requires a little trickery because there is no easy way to tellmj-context-menu
not to callstore.active.focus()
during theunpost()
method for the menu. A better solution would be to add a feature tomj-context-menu
to control that (say via an argument tounpost()
)The
a11y
options weren't being reset by the "reset to defaults" menu item because the values being reset where based on the keys of themenu.settings
object. This PR changes that to use thedefaultSettings
keys instead, and to include thea11y
values in thedefaultSettings
object. Because the explorer can be loaded dynamically (if it is not included in the combined component, or if individual components are loaded separately), thedefaultSettings
may need to be augmented when the explorer is loaded.Finally, the reset code was changed to only reset the values that have changed, and to prevent multiple re-renderings of the document during the reset.
This resolves issue mathjax/MathJax#3310, and fixes several other items not indicated there.