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.

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

Speech Profile Page Improvements #971

Merged
merged 11 commits into from
Oct 8, 2024
Merged

Speech Profile Page Improvements #971

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a5d8db8
ProgressBarWithPercentage floating widget
mdmohsin7 Oct 4, 2024
e69da43
increase length and add no_speech and misc
mdmohsin7 Oct 4, 2024
5038176
Merge branch 'main' into speech-profile
mdmohsin7 Oct 5, 2024
8ab232d
minor improvements
mdmohsin7 Oct 5, 2024
dd1af99
misc
mdmohsin7 Oct 5, 2024
c812cd4
improve ui and reduce word count
mdmohsin7 Oct 5, 2024
31ea9cb
improve indicator positioning
mdmohsin7 Oct 6, 2024
b4a1151
improve sizing on onboarding speech profile
mdmohsin7 Oct 6, 2024
489ea45
comment out MemoryCreatedWidget in onboarding
mdmohsin7 Oct 6, 2024
8e88769
move clearing of speech provider
mdmohsin7 Oct 7, 2024
2c8eb42
Merge branch 'main' into speech-profile
mdmohsin7 Oct 8, 2024
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
75 changes: 46 additions & 29 deletions app/lib/pages/onboarding/speech_profile_widget.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'dart:async';
import 'package:flutter/material.dart';
import 'package:flutter_provider_utilities/flutter_provider_utilities.dart';
import 'package:friend_private/backend/preferences.dart';
import 'package:friend_private/pages/speech_profile/percentage_bar_progress.dart';
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The import of percentage_bar_progress.dart is new. Ensure that this file exists and is accessible from the current file's location. If it doesn't exist or isn't accessible, it will cause a compilation error.

+ import 'package:friend_private/pages/speech_profile/percentage_bar_progress.dart';

import 'package:friend_private/providers/capture_provider.dart';
import 'package:friend_private/providers/speech_profile_provider.dart';
import 'package:friend_private/widgets/dialog.dart';
Expand Down Expand Up @@ -138,6 +139,23 @@ class _SpeechProfileWidgetState extends State<SpeechProfileWidget> with TickerPr
),
barrierDismissible: false,
);
} else if (error == "NO_SPEECH") {
showDialog(
context: context,
builder: (c) => getDialog(
context,
() {
Navigator.pop(context);
// Navigator.pop(context);
},
() {},
'Are you there?',
'We could not detect any speech. Please make sure to speak for at least 10 seconds and not more than 3 minutes.',
okButtonText: 'Ok',
singleButton: true,
),
barrierDismissible: false,
);
Comment on lines +142 to +158
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The error handling for "NO_SPEECH" has been improved. The new dialog provides a clearer message to the user about what went wrong and how to fix it. However, consider using a constant or enum for the error string "NO_SPEECH" to avoid potential typos and improve maintainability.

- } else if (error == "NO_SPEECH") {
+ } else if (error == SpeechErrors.NO_SPEECH) {

}
},
child: Container(
Expand All @@ -148,17 +166,22 @@ class _SpeechProfileWidgetState extends State<SpeechProfileWidget> with TickerPr
height: 10,
),
Padding(
padding: const EdgeInsets.fromLTRB(40, 20, 40, 20),
padding: EdgeInsets.fromLTRB(40, !provider.startedRecording ? 20 : 0, 40, 20),
child: !provider.startedRecording
? const Text(
'Now, Omi needs to learn your voice to be able to recognise you.',
textAlign: TextAlign.center,
style: TextStyle(
color: Colors.white,
fontSize: 20,
height: 1.4,
fontWeight: FontWeight.w400,
),
? const Column(
children: [
Text(
'Omi needs to learn your voice to recognize you',
textAlign: TextAlign.center,
style: TextStyle(
color: Colors.white,
fontSize: 20,
height: 1.4,
fontWeight: FontWeight.w400,
),
),
Text("Note: This only works in English", style: TextStyle(color: Colors.white)),
],
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The padding and text content have been updated. The changes seem fine, but again, consider moving hardcoded strings to a separate constants file or use localization for better maintainability.

)
: LayoutBuilder(
builder: (context, constraints) {
Expand Down Expand Up @@ -295,30 +318,24 @@ class _SpeechProfileWidgetState extends State<SpeechProfileWidget> with TickerPr
: Column(
mainAxisSize: MainAxisSize.min,
children: [
const SizedBox(height: 30),
const SizedBox(height: 20),
provider.percentageCompleted > 0
? const SizedBox()
: const Text(
"Introduce\nyourself",
style: TextStyle(color: Colors.white, fontSize: 24, height: 1.4),
textAlign: TextAlign.center,
),
// const SizedBox(height: 30),

// const SizedBox(height: 18),
ProgressBarWithPercentage(progressValue: provider.percentageCompleted),
const SizedBox(height: 12),
Text(
provider.message,
style: TextStyle(color: Colors.grey.shade300, fontSize: 14, height: 1.4),
textAlign: TextAlign.center,
),
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The progress bar has been replaced with ProgressBarWithPercentage which provides a more detailed progress display. This is a good improvement for user experience. However, the hardcoded string "Introduce\nyourself" should be moved to a constants file or handled through localization.

const SizedBox(height: 18),
Padding(
padding: const EdgeInsets.symmetric(horizontal: 32),
child: Stack(
children: [
LinearProgressIndicator(
value: provider.percentageCompleted,
backgroundColor: Colors.grey.shade300,
valueColor: const AlwaysStoppedAnimation<Color>(Colors.deepPurple),
),
],
),
),
const SizedBox(height: 12),
Text(
'${(provider.percentageCompleted * 100).toInt()}%',
style: const TextStyle(color: Colors.white),
),
],
),
),
Expand Down
164 changes: 80 additions & 84 deletions app/lib/pages/speech_profile/page.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import 'package:gradient_borders/box_borders/gradient_box_border.dart';
import 'package:intercom_flutter/intercom_flutter.dart';
import 'package:provider/provider.dart';

import 'percentage_bar_progress.dart';
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The import statement for percentage_bar_progress.dart has been added. This is necessary to use the new ProgressBarWithPercentage widget in this file.

+ import 'percentage_bar_progress.dart';


Comment on lines +19 to +20
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The import statement for percentage_bar_progress.dart has been added. This is necessary to use the new ProgressBarWithPercentage widget in this file.

+ import 'package:intercom_flutter/intercom_flutter.dart';
+ import 'package:provider/provider.dart';
+ 
+ import 'percentage_bar_progress.dart';

class SpeechProfilePage extends StatefulWidget {
final bool onbording;

Expand Down Expand Up @@ -114,7 +116,7 @@ class _SpeechProfilePageState extends State<SpeechProfilePage> with TickerProvid
Navigator.pop(context);
},
() {},
'Invalid recording detected',
'Multiple speakers detected',
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The error message title has been updated to be more specific about the error. This change improves user experience by providing clearer feedback.

- 'Invalid recording detected',
+ 'Multiple speakers detected',

'It seems like there are multiple speakers in the recording. Please make sure you are in a quiet location and try again.',
okButtonText: 'Try Again',
singleButton: true,
Expand Down Expand Up @@ -208,95 +210,103 @@ class _SpeechProfilePageState extends State<SpeechProfilePage> with TickerProvid
),
body: Stack(
children: [
Align(
const Align(
alignment: Alignment.topCenter,
child: Padding(
padding: EdgeInsets.fromLTRB(16, 8, 16, 0),
padding: EdgeInsets.fromLTRB(24, 8, 24, 0),
child: Column(
children: [
const DeviceAnimationWidget(sizeMultiplier: 0.2, animatedBackground: false),
!provider.startedRecording
? const SizedBox(height: 0)
: const Text(
'Tell your Friend\nabout yourself',
style: TextStyle(
color: Colors.white, fontSize: 18, fontWeight: FontWeight.w500, height: 1.4),
textAlign: TextAlign.center,
),
DeviceAnimationWidget(animatedBackground: true),
],
),
),
),
Align(
alignment: Alignment.center,
child: Padding(
padding: const EdgeInsets.fromLTRB(40, 0, 40, 48),
padding: const EdgeInsets.fromLTRB(40, 40, 40, 48),
child: !provider.startedRecording
? const Text(
'Now, Omi needs to learn your voice to be able to recognise you.',
textAlign: TextAlign.center,
style: TextStyle(
color: Colors.white,
fontSize: 20,
height: 1.4,
fontWeight: FontWeight.w400,
),
? const Column(
mainAxisAlignment: MainAxisAlignment.center,
children: [
SizedBox(height: 10),
Text(
'Omi needs to learn your voice to be able to recognise you.',
textAlign: TextAlign.center,
style: TextStyle(
color: Colors.white,
fontSize: 20,
height: 1.4,
fontWeight: FontWeight.w400,
),
),
SizedBox(height: 20),
Text("Note: This only works in English",
style: TextStyle(color: Colors.white, fontSize: 16)),
],
)
: provider.text.isEmpty
? const DeviceAnimationWidget(
sizeMultiplier: 0.7,
)
: LayoutBuilder(
builder: (context, constraints) {
return ShaderMask(
shaderCallback: (bounds) {
if (provider.text.split(' ').length < 10) {
return const LinearGradient(colors: [Colors.white, Colors.white])
.createShader(bounds);
}
return const LinearGradient(
colors: [Colors.transparent, Colors.white],
stops: [0.0, 0.5],
begin: Alignment.topCenter,
end: Alignment.bottomCenter,
).createShader(bounds);
},
blendMode: BlendMode.dstIn,
child: SizedBox(
height: 120,
child: ListView(
controller: _scrollController,
shrinkWrap: true,
physics: const NeverScrollableScrollPhysics(),
children: [
Text(
provider.text,
textAlign: TextAlign.center,
style: const TextStyle(
color: Colors.white,
fontSize: 20,
fontWeight: FontWeight.w400,
height: 1.5,
? (provider.percentageCompleted > 0
? const SizedBox()
: const Text(
"Introduce\nyourself",
style: TextStyle(color: Colors.white, fontSize: 24, height: 1.4),
textAlign: TextAlign.center,
))
: Padding(
padding: const EdgeInsets.only(top: 80.0),
child: LayoutBuilder(
builder: (context, constraints) {
return ShaderMask(
shaderCallback: (bounds) {
if (provider.text.split(' ').length < 10) {
return const LinearGradient(colors: [Colors.white, Colors.white])
.createShader(bounds);
}
return const LinearGradient(
colors: [Colors.transparent, Colors.white],
stops: [0.0, 0.5],
begin: Alignment.topCenter,
end: Alignment.bottomCenter,
).createShader(bounds);
},
blendMode: BlendMode.dstIn,
child: SizedBox(
height: 130,
child: ListView(
controller: _scrollController,
shrinkWrap: true,
physics: const NeverScrollableScrollPhysics(),
children: [
Text(
provider.text,
textAlign: TextAlign.center,
style: const TextStyle(
color: Colors.white,
fontSize: 20,
fontWeight: FontWeight.w400,
height: 1.5,
),
),
),
],
],
),
),
),
);
},
);
},
),
),
),
Comment on lines +214 to 299
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

This hunk contains several changes related to UI improvements and adjustments. The padding, text styling, and content have been modified to enhance the visual presentation of the SpeechProfilePage widget. A note has also been added to inform users that the feature only works in English. These changes are subjective and depend on the design requirements of the application.

However, there's a potential issue with the hardcoded sizes and dimensions (e.g., EdgeInsets.fromLTRB(24, 8, 24, 0), SizedBox(height: 10), etc.). It's recommended to use relative sizes or responsive layout techniques to ensure the UI looks good across different screen sizes and resolutions.

),
Align(
alignment: Alignment.bottomCenter,
child: Padding(
padding: const EdgeInsets.fromLTRB(24, 0, 24, 48),
padding: const EdgeInsets.fromLTRB(0, 0, 0, 48),
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The padding on the left and right of the bottom widget has been removed. This could affect the layout and appearance of the page. Ensure that this change aligns with the intended design.

- padding: const EdgeInsets.fromLTRB(24, 0, 24, 48),
+ padding: const EdgeInsets.fromLTRB(0, 0, 0, 48),

child: !provider.startedRecording
? Column(
mainAxisAlignment: MainAxisAlignment.end,
children: [
provider.isInitialising
? CircularProgressIndicator(
? const CircularProgressIndicator(
color: Colors.white,
)
: MaterialButton(
Expand Down Expand Up @@ -412,32 +422,18 @@ class _SpeechProfilePageState extends State<SpeechProfilePage> with TickerProvid
: Column(
mainAxisSize: MainAxisSize.min,
children: [
const SizedBox(height: 24),
SizedBox(
width: MediaQuery.sizeOf(context).width * 0.9,
child: ProgressBarWithPercentage(progressValue: provider.percentageCompleted),
),
const SizedBox(height: 18),
Text(
provider.message,
style: TextStyle(color: Colors.grey.shade300, fontSize: 14, height: 1.4),
textAlign: TextAlign.center,
),
const SizedBox(height: 24),
Padding(
padding: const EdgeInsets.symmetric(horizontal: 32),
child: Stack(
children: [
// LinearProgressIndicator(
// backgroundColor: Colors.grey[300],
// valueColor: AlwaysStoppedAnimation<Color>(Colors.grey.withOpacity(0.3)),
// ),
LinearProgressIndicator(
value: provider.percentageCompleted,
backgroundColor:
Colors.grey.shade300, // Make sure background is transparent
valueColor: const AlwaysStoppedAnimation<Color>(Colors.deepPurple),
),
],
),
),
const SizedBox(height: 12),
Text('${(provider.percentageCompleted * 100).toInt()}%',
style: const TextStyle(color: Colors.white)),
const SizedBox(height: 30),
Comment on lines +426 to +437
Copy link

Choose a reason for hiding this comment

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

Image description Entelligence.AI

The old progress bar (LinearProgressIndicator) has been replaced with a new custom widget (ProgressBarWithPercentage). This new widget provides a more detailed progress display, which can improve user experience. However, it's important to thoroughly test this new widget to ensure it behaves as expected in all scenarios.

- LinearProgressIndicator(
-   value: provider.percentageCompleted,
-   backgroundColor: Colors.grey.shade300,
-   valueColor: const AlwaysStoppedAnimation<Color>(Colors.deepPurple),
- ),
+ SizedBox(
+   width: MediaQuery.sizeOf(context).width * 0.9,
+   child: ProgressBarWithPercentage(progressValue: provider.percentageCompleted),
+ ),

],
),
),
Expand Down
Loading
Loading