8000 Cleanup of VoiceSearchWidget by felipeerias · Pull Request #1886 · Igalia/wolvic · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Cleanup of VoiceSearchWidget #1886

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

felipeerias
Copy link
Collaborator

Simplify the code of VoiceSearchWidget and make it respond better to state changes.

The main change is to add a new State class that
represents the state of the view and its associated message for the user. This allows us to simplify the layout while preserving the existing functionality.

@felipeerias felipeerias force-pushed the felipeerias/cleanupVoiceSearchWidget branch 2 times, most recently from 4cf600b to cd288fd Compare July 2, 2025 09:25
Additional checks to ensure that the voice recognition
system is always in a consistent state.
Simplify the code of VoiceSearchWidget and make it
respond better to state changes.

The main change is to add a new State class that
represents the state of the view and its associated
message for the user. This allows us to simplify the
layout while preserving the existing functionality.
@felipeerias felipeerias force-pushed the felipeerias/cleanupVoiceSearchWidget branch from cd288fd to dfff9e1 Compare July 3, 2025 06:11
@@ -219,16 +231,17 @@ public void onCanceled() {
@Override
public void onError(@ErrorType int errorType, @Nullable String error) {
Log.e(LOGTAG, "===> ERROR: " + error);
setResultState(errorType);
stopVoiceSearch();
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to call this in setState() instead? We could easily check if the new state is an error condition and stop the voice search there. Or are there cases in which setting an error state does not imply stopping the voice search?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently, all error states stop the voice search.

Some errors are recoverable (e.g. the user remained silent) and some others are actual problems (e.g. the network is unavailable). If we continue evolving the UI, maybe in some cases we could try restarting the voice search.

In any case, the reason for having these two calls here is that setStateforError() only encapsulates UI changes.

Copy link
Member
@svillar svillar left a comment

Choose a reason for hiding this comment

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

Just a few comments

@felipeerias felipeerias requested a review from svillar July 4, 2025 11:52
Copy link
Member
@svillar svillar left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

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
0