Move a11y attribute and property strings to be taken from shared objects - #1551
Open
dpvc wants to merge 2 commits into
Open
Move a11y attribute and property strings to be taken from shared objects#1551dpvc wants to merge 2 commits into
dpvc wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR moves the strings used for the various
data-*attributes in the a11y components into objects, similar to theSemAttrarray used in the speech processing. This both shortens the references, and guarantees that they are the same. It also makes it possible to change them easier, if that is desired, as changes only need to be made in one place.We also fix three other issues that were found while making the string changes:
__locales__files weren't being includes in theMathJax._object for some of the components, so theirconfig.jsonfiles were modified to do that explicitly.collapsedComplecityproperty being checked that is never set anywhere in the code base. I believe that it was left over from an earlier usage and that this should have been changed tocollapse-complexity, so I've change it to that.feature/common-structurebranch somewhere (perhaps in how the regions start/stop or update), but I didn't track it down specifically. Instead, an easy fix is to stop the loop that places the regions when you get to the current region. I also simplified the loops.This touches a lot of files, but most of the changes are just substituting the object references for the literal strings. Some spacing in comments is also adjusted for alignment.
Details
The component config files are modoified to load load the files that define the strings, and to load
__locales__when that isn't already being done (so theirComponent.tsfiles will be available to be shared). Because the semantic-enrich component is shared by all thea11ycomponents, the strings that are shared by more than one components are included in thea11y/semantic-enrichcomponent, so that the stings aren't duplicated in each component. Strings that are used in only one component are included only in that one component. (This is true for the complexity and explorer components).Some strings, like the maction strings used for marking the mactions from the complexity component, are used in components that aren't based on the semantic enrich component, and so they will include those strings themselves. This includes the
ui/menucomponent, and the two output components, since themactionsupport needs to use the maction strings, and the commonscriptbasewrapper needs two semantic attributes.This does mean there is a small amount of duplication due to those strings needing to be used in several unrelated components, but the cost is small, and while the duplication appears in the files, it will not take up more memory as the strings will be stored in the same place, so a later component overwrites the original strings with the second copy of the same strings.
There are two alternaitve approaches that could be use to avoid any duplication, however. The first would be to move the strings to the
corecomponent, so they are available to be shared with all other components. The disadvantage is that this means strings that if the strings will be loaded even if the a11y components are not. That would not affect those who use a combined configuration (liketex-chtml.js), but would affect those who load components individually and don't include any a11y components.The second approach would be to use
export const enumobjects rather thanexport constobjects. That would cause Typescript to inline the strings rather than include object references. That means there are no objects to be shared as the strings are inserted into the files directly at compile time. This would make the files be essentially the same as they are now (with explicit strings everywhere), so nothing needs to be shared. The downside here is that the strings are no longer available inMathJax._for other code to reference, so run-time patches would need to use the explicit strings. Also, the strings themselves are often longer than the object references, which mens we don't get any file-size savings that we do by using the common objects. (E.g.,'data-semantic-id'is longer thanSEM.ID, and we save 12 characters in the file that needs to be downloaded to the browser for every reference to this string. Not a huge savings, but still, any savings in file size is useful.In the
components/mjs/a11y/complexity/config.jsonfile,semantic-enrich.jswas being incorrectly included, so has been removed. The complexity component has the semantic-enrich component as a dependency, so we don't want to include it directly.The explorer component wasn't loading the
a11y/__locales__into themathjax._object, so that has been included here.The semantic-enrich configuration now loads the common strings (and its
_locales__via thesemantic-enrichsubdirectory).The
adaptors/linkedomneeds to have its__locales__included (not really related to the string changes, but I found it when the__locales__for the a11y components weren't being loaded, so include it here as well). The same for theinput/mmlandinput/texcomponents.As mentioned above, the output components need the semantic-enrich and maction strings for the
scriptbaseandmactionwrappers. They aren't shared, since these components don't havesemantic-enrichas a dependency, so mush be included explicitly so that they appear in theMathJax._object are aren't duplicated when both components are loaded.Similarly, the
ui/menucomponent needs some of the attribute strings, and it also was not merging the__locales__values into the sharedMathJax._object, so they are included here.The changes in
a11y/complexity/collapse.tsare just the replacement of the strings with the object references. The strings themselves are ina11y/complexity/strings.ts. Note thatdata-semantic-complexityis changed todata-collapse-complexity, as it is not an SRE attribute. This is a potential breaking change, but I doubt anyone is using these values. If they are, they can change the attribute back using theMathJax.loader['a11y/complexity'].ready()function, as long as we use actual objects, notcont enum, for these strings.The changes to
visitor.tsare just the string substations and some alignment adjustments in the comments.For
explorer.ts, we change the strings, and also use a newSAVED_HREFstring fromKeyExplorer.tsfor the attribute that stored the saveshrefattributes during exploration.The
ATTRarray already in use inHighlighter.tsis renamesHILITEto make it more clear where it comes from, and is now stored in a separatestrings.tsfile for the explorer. TheExplorer.tsfile is modified to use the new array name and location.The
ExplorerPool.tsfile has its semantic strings exchanged for theSEMobject.In
KeyExplorer.ts, most changes are just the string replacements. A newSAVED_HREFconstant is used for the attribute used to save the originalhrefattributes. (I suppose this could have been in theexplorer/strings.tsfile, perhaps asHREF.SAVED; I can do that if you want that for consistency). The change infirstNode()is to eliminate the need for a redundantreturn node..querySelector(nav)call.The
MouseExplorer.tsjust has strings replaced by the object references.In
Region.ts, we defined a set of attributes that are used internally for marking the magnification regions as clones (so that CSS can target the magnification and not the original expression). We also adjust thestackRegions()method, as mentioned above, so that we stop stacking when we get to the current region rather than including all the regions below it in its position. I'm not sure why this worked in the past, but suspect that the regions were removed and re-added, so that there were not any below it. In any case, this now works, and the loop can be simplified to usefor-ofrather than an index. The other changes are just replacing the strings with the object references.The
TreeExplorer.tschanges are just the string replacements.The
explorer/strings.tsfile just defines theHILITEstrings (the oldATTRlist, plus aPREFIX.The changes in
semantic-enrich.tsare just the replacement of strings with object references.The
semantic-enrich/maction.tsfile defines the strings for maction attributes used for collapsing sub expressions. The names now usedata-maction-rather thandata-collapse-for the group and group id attributes. These are not breaking changes, since those attributes were introduced infeature/common-structure, which hasn't been merged yet.The
semantic-enrich/strings.tsfile defines thedata-semantic-strings used in thea11ycomponents and elsewhere. Note that there is already aSemAttrarray inSpeechUtil.tsthat holds somedata-semantic-attributes, and it would be possible to combine these arrays. They are used in the GeneratorPool, WebWorker, and KeyExplorer files, so could be added to the newSEMarray easily.There remaining files are just strings replacements with objects references, and the
speech/strings.tsdefinition file.