Skip to content

Search fix #2265

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.

Sign up for GitHub

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

Merged
beastoin merged 3 commits into from
May 4, 2025
Merged

Search fix #2265

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2303f7c
do not show LanguageSelectionDialog if the user hasn't completed onbo…
mdmohsin7 Apr 26, 2025
7ec3e29
id for transcript_segment can be null
mdmohsin7 Apr 26, 2025
c349b70
revert the onboarding completed check for language dialog
mdmohsin7 May 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/lib/backend/schema/transcript_segment.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class TranscriptSegment {
// Factory constructor to create a new Message instance from a map
factory TranscriptSegment.fromJson(Map<String, dynamic> json) {
return TranscriptSegment(
id: json['id'] as String,
id: (json['id'] ?? '') as String,
text: json['text'] as String,
speaker: (json['speaker'] ?? 'SPEAKER_00') as String,
isUser: (json['is_user'] ?? false) as bool,
Expand Down
2 changes: 1 addition & 1 deletion app/lib/providers/home_provider.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class HomeProvider extends ChangeNotifier {
}

void showLanguageDialogIfNeeded(BuildContext context) {
if (!hasSetPrimaryLanguage) {
if (!hasSetPrimaryLanguage && SharedPreferencesUtil().onboardingCompleted) {

@beastoin beastoin Apr 28, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why do we need && SharedPreferencesUtil().onboardingCompleted sir ?

since primary language is a required information.

@mdmohsin7 mdmohsin7 Apr 28, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Previously sometimes the language selection dialog was being shown even during the onboarding flow (had it happen multiple times while debugging), but we already have the language page in the onboarding flow so I don't think we should show it twice on the onboarding flow

@beastoin beastoin Apr 29, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

oops ~ even to prevent the duplication of the dialog displaying, the logic of the primary language should not be changed, instead you should find another way to fix it.

just simple like this, if the user didn't set primary language before, show the dialog.

your code's failure case: if the user just updated the app to the new version, they didnt set the primary language yet, but they already finish the onboarding flow, right. your code will break the above simple logic of the primary language setting.

@mdmohsin7

@mdmohsin7 mdmohsin7 May 3, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Understood! reverted the change for now. Will check this separately

@beastoin

LanguageSelectionDialog.show(context, isRequired: true);
}
}
Expand Down