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

Add ability to not show latex attributes, and reorganize settings menu #1017

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Nov 11, 2023

This PR adds a menu option to remove the data-latex and data-latexItem attributes 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 the MmlVisitor so the attributes are never added, rather than using a string replacement after the fact.

@dpvc dpvc requested a review from zorkow November 11, 2023 20:08
@dpvc dpvc added this to the v4.0 milestone Nov 11, 2023
Copy link
Member

@zorkow zorkow left a 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))$/)
Copy link
Member

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.)

Copy link
Member Author

@dpvc dpvc Nov 20, 2023

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?

Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Sounds good.

@dpvc dpvc changed the title Add ability to no show latex attributes, and reorganize settings menu Add ability to not show latex attributes, and reorganize settings menu Nov 20, 2023
Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

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

lgtm.

@dpvc dpvc merged commit c894001 into develop Nov 28, 2023
@dpvc dpvc deleted the menu-filters branch November 28, 2023 13:37
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.

2 participants