-
Notifications
You must be signed in to change notification settings - Fork 214
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
Fix/explorer rules #1079
Fix/explorer rules #1079
Conversation
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 change suggestion to simplify the speechRules
definition, but other than that looks good.
ts/ui/menu/Menu.ts
Outdated
{ | ||
name: 'speechRules', | ||
getter: () => { | ||
return this.settings['speechRules']; | ||
}, | ||
setter: (value: string) => { | ||
const [domain, style] = value.split('-'); | ||
this.settings['speechRules'] = value; | ||
this.document.options.sre.domain = domain; | ||
this.document.options.sre.style = style; | ||
this.rerender(STATE.COMPILED); | ||
this.saveUserSettings(); | ||
} | ||
}, |
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 should be able to use this.variable()
rather than this.a11yvar()
to define this, so you don't have to do the getter
and setter by hand (and won't need to include lines 517 and 521, which are part of what this.variable()
does. So
this.variable<string>('speechRules', value => {
const [domain, style] = value.split('-');
this.document.options.sre.domain = domain;
this.document.options.sre.style = style;
this.rerender(STATE.COMPILED);
}),
should do it (that is, changing a11yvar
to variable
and adding the rerender()
call to the original lines 510 to 514 should be enough).
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.
I tried that and it not work. And it drove me crazy, so I decided to stick with the pedestrian version.
But I will try again. Maybe I was just too thick the other day.
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.
I just tried this out, and it seemed to work fine for me. Give it another try, and see if you have any problems.
Reenables the change of rule sets and preferences for speech generation.
speechRules
.mathspeek
that lead to incorrect speech computation.mathitem
.Still two issues: