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

Don't update state prematurely in showTranslationFor() #77

Merged
merged 1 commit into from
May 25, 2022

Conversation

xiaoyifang
Copy link
Owner

MainWindow::showTranslationFor() overloads disable the "Pronounce Word"
action, then call ArticleView::showDefinition(). And then immediately
update pronounce availability, Found in Dictionaries list, Back and
Forward buttons. Since ArticleView::showDefinition() loads the requested
page asynchronously, the previous page is still current. Therefore the
"Pronounce Word" action is immediately re-enabled (if the still-current
article has sounds), the other state updates have no effect whatsoever.
Once the new page is loaded, the state is updated again in
MainWindow::pageLoaded() - this time with the desired effect.

So the only effect of the state updates in
MainWindow::showTranslationFor() is to revert the intentional disabling
of the "Pronounce Word" action. Plus waste some CPU time. The
pronunciation-disabling behavior looks better to me and is consistent
with the scan popup's behavior (which immediately hides the
"Pronounce Word" button).

MainWindow::showTranslationFor() overloads disable the "Pronounce Word"
action, then call ArticleView::showDefinition(). And then immediately
update pronounce availability, Found in Dictionaries list, Back and
Forward buttons. Since ArticleView::showDefinition() loads the requested
page asynchronously, the previous page is still current. Therefore the
"Pronounce Word" action is immediately re-enabled (if the still-current
article has sounds), the other state updates have no effect whatsoever.
Once the new page is loaded, the state is updated again in
MainWindow::pageLoaded() - this time with the desired effect.

So the only effect of the state updates in
MainWindow::showTranslationFor() is to revert the intentional disabling
of the "Pronounce Word" action. Plus waste some CPU time. The
pronunciation-disabling behavior looks better to me and is consistent
with the scan popup's behavior (which immediately hides the
"Pronounce Word" button).
@xiaoyifang xiaoyifang merged commit e241cbe into xiaoyifang:staged May 25, 2022
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