Skip to content

Conversation

@dpvc
Copy link
Member

@dpvc dpvc commented Apr 4, 2024

This PR fixes several issues with the new menus (as discussed in our meeting today).

The first is switching the semantic enrichment on and off, which didn't turn on/off the speech and Braille. This was due to now setting the enableSpeech, enableBraille, and enableExplorer values, which this now does in lines 941 and 942 of Menu.ts in the setAccessibilityMenus() function. Some initialization was moved from the initMenu() function to applySettings() (old lines 687 to 689 moved to new 824 to 826, with old lines 837 through 837 being removed, as the setAccessibilityMenus() call now sets those values properly.

The enableExplorer in old line 1004 is not needed, as the setAccessibilityMenus() now handled that (new line 942).

Old lines 1019 and 1020 are adjusted to avoid running this.rerender() twice.

The second issue is that the Hover and Flame highlighting options are dependent on Collapsible Math being set. We talked about disabling these when Collapsible Math is unchecked, but since there isn't an obvious connection between the two, this PR instead causes Collapsible Math to be checked automatically when either Hover or Flame is selected, and the highlighting is reset to None if Collapsible Math is unchecked. That allows you top select the highlighting witty having to know the connection to Collapsible Math.

Finally, I changed the explicit setter/getter to use variable() as we discussed. This seems to work for me, so check if you have any problems.

@dpvc dpvc requested a review from zorkow April 4, 2024 19:12
@dpvc dpvc added this to the v4.0 milestone Apr 4, 2024
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.

Thanks for sorting out the variable. I'll merge straight away.

@zorkow zorkow merged commit 10a5ec8 into fix/explorer_rules Apr 8, 2024
@zorkow zorkow deleted the menu-fixes branch April 8, 2024 17:22
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