Skip to content

Search fix#2265

Merged
beastoin merged 3 commits intomainfrom
search-other-fixes
May 4, 2025
Merged

Search fix#2265
beastoin merged 3 commits intomainfrom
search-other-fixes

Conversation

@mdmohsin7
Copy link
Member

@mdmohsin7 mdmohsin7 commented Apr 26, 2025

  • fix search not working due to id in transcript_segment being null for manually added convos
  • do not show language dialog if the user hasn't onboarded yet (because during onboarding there already is a screen for language selection)


void showLanguageDialogIfNeeded(BuildContext context) {
if (!hasSetPrimaryLanguage) {
if (!hasSetPrimaryLanguage && SharedPreferencesUtil().onboardingCompleted) {
Copy link
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.

Copy link
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

Copy link
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

Copy link
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

@vercel
Copy link

vercel bot commented May 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
omi ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 3, 2025 1:07pm

@mdmohsin7 mdmohsin7 changed the title Search and other fixes Search fix May 3, 2025
@beastoin beastoin merged commit 142ee18 into main May 4, 2025
3 checks passed
@beastoin beastoin deleted the search-other-fixes branch May 4, 2025 09:41
@beastoin
Copy link
Collaborator

beastoin commented May 4, 2025

lgtm @mdmohsin7

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