-
Notifications
You must be signed in to change notification settings - Fork 234
Add ability to not show latex attributes, and reorganize settings menu #1017
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
Conversation
zorkow
left a comment
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 point of discussion.
| } | ||
| if (this.options.filterSRE) { | ||
| const keys = Object.keys(list).filter( | ||
| id => id.match(/^(?:data-semantic-.*?|role|aria-(?:level|posinset|setsize))$/) |
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.
What if we parse from MathML and some of these aria attributes are already in the incoming element?
(I suppose we would have overwritten them at this point anyway.)
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.
This filtering only occurs in the Show Math As or Copy Math As menus. Nothing is changed internally. This filtering is controlled by menu options, so the user can turn off the filtering in the case where retaining those values is desired. Does that put you more at ease?
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.
Note that this MmlVisitor is in the ts/ui/menu directory, not the one in ts/core/MmlTree, so it only affects the menu output.
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.
Thanks. Sounds good.
zorkow
left a comment
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.
lgtm.
This PR adds a menu option to remove the
data-latexanddata-latexItemattributes from the "Show Math As MathML" menu item, as these can get quite long, making it hard to see the MathML structure. We also reorganize the Math Settings submenu to push the MathML filters into an additional submenu level. Finally, we move the filtering of the SRE attributes into theMmlVisitorso the attributes are never added, rather than using a string replacement after the fact.