Menu tab by AbedrahmanYassen · Pull Request #360 · CollActionteam/collaction_app

AbedrahmanYassen · 2023-02-12T17:25:44Z

I have made the menu tab and used the logic at the settings screen in this tab, after that I have deleted the settings screen.

…ction_app into menu_tab � Conflicts: � pubspec.lock

Xazin · 2023-02-12T18:49:45Z

Seems the analyzer is complaining.

In VSCode, it's a good idea to enable the "Format file on Save" option, such that the dart formatter will always make sure your files are formatted properly.

Xazin · 2023-02-12T18:50:05Z

lib/main.dart

@@ -1,4 +1,4 @@
-import 'dart:async';
+import  'dart:async';


Why another space?

Xazin · 2023-02-12T18:51:09Z

lib/presentation/menu/menu_screen.dart

+  Widget _buildSubTitle(
+      {required String subTitle,
+      double paddingTop = 0.0,
+      double paddingBottom = 0.0}) {


Comma for formatting

Xazin · 2023-02-12T18:51:27Z

lib/presentation/menu/menu_screen.dart

+class MenuPage extends StatefulWidget {
+  const MenuPage({Key? key}) : super(key: key);
+
+  @override
+  State<MenuPage> createState() => _MenuTabState();
+}
+
+class _MenuTabState extends State<MenuPage> {
+  @override
+  Widget build(BuildContext context) {


No reason for this being a Statefulwidget, can be stateless.

Xazin · 2023-02-12T18:52:35Z

lib/presentation/menu/menu_screen.dart

+                        Row(
+                          mainAxisAlignment: MainAxisAlignment.spaceBetween,
+                          crossAxisAlignment: CrossAxisAlignment.center,
+                          children: [
+                            Text(
+                              "Menu",
+                              style: TextStyle(
+                                fontSize: 28,
+                                fontFamily: "Rubik",
+                                fontWeight: FontWeight.w700,
+                              ),
+                            ),
+                            IconButton(
+                              onPressed: () {},
+                              icon: Icon(
+                                Icons.search,
+                                color: kPrimaryColor600,
+                                size: 21.08,
+                              ),
+                            ),
+                          ],
+                        ),


This should most likely be a common widget. Best to extract it.

Xazin · 2023-02-12T18:52:45Z

lib/presentation/menu/menu_screen.dart

+                              "Menu",
+                              style: TextStyle(
+                                fontSize: 28,
+                                fontFamily: "Rubik",


No need to define fontFamily

Xazin · 2023-02-12T18:53:36Z

lib/presentation/menu/menu_screen.dart

+                                    ),
+                                    decoration: BoxDecoration(
+                                        color: Colors.white,
+                                        shape: BoxShape.circle),


Comma for Formatting

Xazin · 2023-02-12T18:53:58Z

lib/presentation/menu/menu_screen.dart

+                        SizedBox(
+                          height: 30,
+                        ),
+                        Container(


This should be a Widget itself as well.

Xazin · 2023-02-12T18:54:13Z

lib/presentation/menu/menu_screen.dart

+                    ),
+                  ),
+                  _buildSubTitle(
+                      subTitle: "Account", paddingTop: 38.5, paddingBottom: 20),


Comma for formatting

Xazin · 2023-02-12T18:56:01Z

lib/presentation/menu/widgets/avatar_info.dart

+class _AvatarAndInfoState extends State<AvatarAndInfo> {
+  late final IUserRepository _userRepository;
+  File? _image;
+  @override
+  void initState() {
+    super.initState();
+    _userRepository = getIt<IUserRepository>();
+  }
+  @override
+  Widget build(BuildContext context) {


Would love to see some line breaks..

Xazin · 2023-02-12T18:56:09Z

lib/presentation/menu/widgets/avatar_info.dart

+        // ProfilePicture(
+        //   maxRadius: 40,
+        //   profileImage:
+        //       '${dotenv.get('BASE_STATIC_ENDPOINT_URL')}/${widget.pictureUrl}',
+        // ),


Xazin · 2023-02-12T18:56:44Z

lib/presentation/menu/widgets/avatar_info.dart

+          profileImage: widget.pictureUrl !=
+              null
+              ? '${dotenv.get('BASE_STATIC_ENDPOINT_URL')}/${widget.pictureUrl}'
+              : null,


Null check but if null then you return null? Look into ?? operator.

Xazin · 2023-02-12T18:58:15Z

lib/presentation/menu/widgets/avatar_info.dart

+            },stream:     _userRepository.observeUser()
+    ,),


Formatting?

Xazin · 2023-02-12T19:00:36Z

lib/presentation/menu/widgets/avatar_info.dart

+              } else {
+                return const Text('...');
+              }


No need for the else, just do:

if (snapshot.hasData) { return Text(snapshot.data!.phoneNumber!, style: phoneNumberTextStyle); } return const Text('...', style: phoneNumberTextStyle);

Xazin · 2023-02-12T19:01:26Z

lib/presentation/menu/widgets/build_information_tile.dart

@@ -15,7 +15,7 @@ class BuildInformationTile extends StatelessWidget {
        const SizedBox(height: 50),
        const SizedBox(
          width: 56,
-          child: Image(image: AssetImage('assets/images/build_info.png')),


If we're not using build_info.png anywhere else, we can delete it from the assets folder.

Xazin · 2023-02-12T19:02:41Z

lib/presentation/menu/widgets/legal_info_policy_widget.dart

+      {Key? key,
+        required this.onTap,
+        required this.iconWidget,
+        required this.label})


comma for formatting

Xazin · 2023-02-12T19:02:47Z

lib/presentation/menu/widgets/legal_info_policy_widget.dart

+import '../../themes/constants.dart';
+
+class LegalInfoAndPoliciesWidget extends StatefulWidget {
+  final onTap;


Needs to be typed

Xazin · 2023-02-12T19:03:28Z

lib/presentation/menu/widgets/legal_info_policy_widget.dart

+        onTap:(){
+          widget.onTap();
+        },


Suggested change

onTap:(){

widget.onTap();

},

onTap:() => widget.onTap(),

Xazin · 2023-02-12T19:04:16Z

lib/presentation/themes/constants.dart

+const Color kPrimaryColor600 = Color(0xFF2EB494);
 const TextStyle kCaption1 = TextStyle(


Line break?

Xazin · 2023-02-12T19:04:26Z

lib/presentation/themes/constants.dart

+
+
+


Too many line breaks, use the formatter

Xazin · 2023-02-12T19:04:51Z

pubspec.yaml

  flutter_bloc: ^8.1.1
  flutter_dotenv: ^5.0.2
  freezed_annotation: ^2.2.0
+  fvm: ^2.4.1


What is this? We don't use it nor need it?

Xazin · 2023-02-12T19:04:58Z

pubspec.yaml

+dependency_overrides:
+  firebase_core_platform_interface: 4.5.1


Xazin · 2023-02-14T17:57:54Z

Formatter still fails

Xazin · 2023-02-28T16:18:18Z

@AbedrahmanYassen How is it going?

I made sure all the screens are formatted, what can be the problem?

It's specifically complaining about Changed lib/presentation/home/home_screen.dart Changed lib/presentation/routes/app_routes.dart - You can try to run the dart format command and see.

Tests are also failing

fix: add manual logging to auth repo

Xazin

Why did you remove .env.example? It's crucial for it to be there.

Xazin · 2023-03-10T08:24:01Z

lib/presentation/menu/menu_screen.dart

+    );
+  }
+
+  Widget _buildSubTitle({


Don't use methods returning Widgets if there is no reason to do so.

Make it into a StatelessWidget with a constant constructor. Functions and methods returning widgets that are used in Build methods are a pitfall for useless rebuilding.

Xazin · 2023-03-10T08:26:04Z

lib/presentation/menu/menu_screen.dart

+                              SizedBox(
+                                width: 306,
+                                child: Text(
+                                  "View and update your account  and contact information that is associated with your CollAction account.",
+                                  style: TextStyle(
+                                      fontSize: 12,
+                                      color: kPrimaryColor300,
+                                      fontWeight: FontWeight.w400),
+                                ),
+                              ),


Why are you using a fixed width here (why even a sizedbox?)? This won't make it look well on devices.

I will fix it soon, sorry I got exams 😥.

…ncy, make readable

Xazin · 2023-03-16T07:22:09Z

I've pushed a commit.

There were a lot of discrepancies according to Design. Take a look at my commit and it's changes, and have the figma beside you when you look it through.

Some of the generic mistakes:

Don't use fixed sizing for Dynamically Sized Widgets
When using BLoC, remember to account for loading state, otherwise you'll get a grey/red screen due to null guarantee ! where not applicable
Size by spacing, correct spacing, font size, icon sizes, makes a bigger chance it's going to look according to design

And some other minor things.

wizlif

@AbedrahmanYassen The design looks good, just add some fixes.

lib/presentation/menu/menu_screen.dart

wizlif · 2023-03-17T19:13:08Z

lib/presentation/menu/widgets/ca_menu_item.dart

+        padding: const EdgeInsets.only(left: 20, right: 54) +
+            const EdgeInsets.symmetric(vertical: 10),


Same for this. Use EdgeInsets.fromLTRB

wizlif · 2023-03-17T19:15:27Z

lib/presentation/menu/widgets/ca_menu_item.dart

+                    style: TextStyle(
+                      fontSize: 14,
+                      fontWeight: FontWeight.w500,
+                      color: kPrimaryColor300,
+                    ),


It's not easily visible but we could take into consideration the line height from figma just to make it pixel perfect.

Same for other text styles.

how much line-height should I add ?

Check the figma

wizlif · 2023-03-17T19:24:34Z

lib/presentation/menu/widgets/header_bar.dart

+
+import '../../themes/constants.dart';
+
+class HeaderBar extends StatelessWidget {


Retire this after implementation of AppBar instead.

We have the implementation of the search bar, I think It's easier to implement the search bar, if It's like this not an AppBar, What do you think?

wizlif · 2023-03-17T19:25:34Z

lib/presentation/menu/widgets/user_info_and_avatar.dart

+          children: [
+            AvatarAndInfo(
+              avatar: avatar,
+              phoneNumber: '+31 612345678',


Add a TODO: To get the phone number from the profile.

There is no need to get the phone number from the profile, once the user has signed in, the phone number will be showed.

wizlif · 2023-03-17T19:27:18Z

lib/presentation/profile/widget/profile_picture.dart

    if (profileImage != null && imageProvider == null) {
      imageProvider = NetworkImage(
-        profileImage!,
+        "${dotenv.env['BASE_STATIC_ENDPOINT_URL']}/$profileImage",


Create a config file with static url instead of directly using dotenv.

This is also the reason the profile_picture_test.dart is failing

wizlif · 2023-03-18T05:42:55Z

lib/application/settings/build_information/build_information_bloc.dart

    on<BuildInformationEvent>((event, emit) async {
      await event.when(
        fetch: () async {
-          emit(const BuildInformationState.loading());


Any particular reason for removing the loading state ??

It's also the reason the build_information_test and building_information_bloc_test are failing

This one is tricky, it should be the initial state of the BLOC

The blocTest expects only emitted values when an event is added. So skipping the initial state.

Co-authored-by: Isaac Obella <wizlif@users.noreply.github.com>

…a Widget

codecov · 2023-04-14T23:39:53Z

Codecov Report

Merging #360 (5012b82) into development (1e9981e) will decrease coverage by 9.39%.
The diff coverage is 15.94%.

@@               Coverage Diff               @@
##           development     #360      +/-   ##
===============================================
- Coverage        65.88%   56.49%   -9.39%     
===============================================
  Files              161      157       -4     
  Lines             4095     4064      -31     
===============================================
- Hits              2698     2296     -402     
- Misses            1397     1768     +371

Impacted Files	Coverage Δ
lib/application/auth/auth_bloc.dart	`0.00% <ø> (-49.40%)`	⬇️
.../application/participation/participation_bloc.dart	`0.00% <0.00%> (ø)`
lib/application/user/profile/profile_event.dart	`0.00% <0.00%> (ø)`
...application/user/profile_tab/profile_tab_bloc.dart	`0.00% <0.00%> (-87.50%)`	⬇️
lib/core/config/network_config.dart	`50.00% <ø> (-16.67%)`	⬇️
lib/domain/user/user.dart	`100.00% <ø> (ø)`
.../infrastructure/auth/firebase_auth_repository.dart	`45.16% <0.00%> (+0.95%)`	⬆️
...astructure/crowdaction/crowdaction_repository.dart	`0.00% <0.00%> (-100.00%)`	⬇️
...ucture/participation/participation_repository.dart	`0.00% <0.00%> (-100.00%)`	⬇️
lib/presentation/auth/auth_screen.dart	`1.96% <0.00%> (-0.04%)`	⬇️
... and 34 more

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

AbedrahmanYassen added 4 commits February 7, 2023 10:39

Finishing half of the UI of the menu tab

ada75b8

Finishing the Menu screen as a UI.

1b02c31

Finishing the menu tab, and removing the settings screen.

a176209

Merge branch 'development' of https://github.com/CollActionteam/colla…

c0fd484

…ction_app into menu_tab � Conflicts: � pubspec.lock

AbedrahmanYassen requested a review from Xazin February 12, 2023 18:15

Xazin reviewed Feb 12, 2023

View reviewed changes

lib/main.dart Outdated

@@ -1,4 +1,4 @@

import 'dart:async';

import 'dart:async';

Copy link

Member

Xazin Feb 12, 2023

Choose a reason for hiding this comment

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

Why another space?

Xazin reviewed Feb 12, 2023

View reviewed changes

lib/presentation/menu/menu_screen.dart Outdated

SizedBox(

height: 30,

),

Container(

Copy link

Member

Xazin Feb 12, 2023

Choose a reason for hiding this comment

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

This should be a Widget itself as well.

Xazin reviewed Feb 12, 2023

View reviewed changes

lib/presentation/themes/constants.dart Outdated

Comment on lines 56 to 58

Copy link

Member

Xazin Feb 12, 2023

Choose a reason for hiding this comment

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

Too many line breaks, use the formatter

Xazin reviewed Feb 12, 2023

View reviewed changes

pubspec.yaml Outdated

Comment on lines 81 to 82

dependency_overrides:

firebase_core_platform_interface: 4.5.1

Copy link

Member

Xazin Feb 12, 2023

Choose a reason for hiding this comment

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

No need?

AbedrahmanYassen added 2 commits February 14, 2023 19:47

Solving the the inconsistencies.

b77f84c

Solving the the inconsistencies.

7ddabec

Xazin force-pushed the development branch from 1bddb8d to 9473c3d Compare March 1, 2023 11:15

Xazin and others added 4 commits March 4, 2023 19:49

fix: add manual logging to auth repo

cf9b0b0

Deleting settings folder at the testing part

8d903e3

Deleting settings folder at the testing part

fa2d650

Merge pull request #371 from CollActionteam/fix/manual-logging-auth-repo

649893f

fix: add manual logging to auth repo

Xazin force-pushed the development branch from 8906c8d to 649893f Compare March 4, 2023 22:45

Xazin and others added 7 commits March 5, 2023 00:15

chore: upgrade gradle build tools and dependencies

023d67e

Finishing half of the UI of the menu tab

2aa649c

Finishing the Menu screen as a UI.

2814954

Finishing the menu tab, and removing the settings screen.

51705a2

Solving the the inconsistencies.

6e8121f

Solving the the inconsistencies.

88a9f1b

Deleting settings folder at the testing part

061f6e8

Xazin force-pushed the menu_tab branch from fa2d650 to 061f6e8 Compare March 10, 2023 08:17

Xazin reviewed Mar 10, 2023

View reviewed changes

refactor: amend according to design, fix connecitivty, remove redunda…

f727c0c

…ncy, make readable

Xazin requested a review from wizlif March 16, 2023 07:22

wizlif reviewed Mar 18, 2023

View reviewed changes

Xazin force-pushed the development branch from 1d1649c to bc76c10 Compare March 23, 2023 13:15

AbedrahmanYassen and others added 5 commits April 15, 2023 00:45

Update lib/presentation/menu/menu_screen.dart

1309fa0

Co-authored-by: Isaac Obella <wizlif@users.noreply.github.com>

Update lib/presentation/menu/menu_screen.dart

a74ca0b

Co-authored-by: Isaac Obella <wizlif@users.noreply.github.com>

Merge branch 'development' into menu_tab

8b9fe77

I have added the .env.example and edited the _subTitle function into …

0575f6a

…a Widget

I have added the .env.example and edited the _subTitle function into …

5012b82

…a Widget

Edamijueda mentioned this pull request Dec 11, 2023

[FEATURE]: Menu tab: refactor #412

Open

		const Color kPrimaryColor600 = Color(0xFF2EB494);
		const TextStyle kCaption1 = TextStyle(

		dependency_overrides:
		firebase_core_platform_interface: 4.5.1

		padding: const EdgeInsets.only(left: 20, right: 54) +
		const EdgeInsets.symmetric(vertical: 10),


		import '../../themes/constants.dart';

		class HeaderBar extends StatelessWidget {

Conversation

AbedrahmanYassen commented Feb 12, 2023

Uh oh!

Xazin commented Feb 12, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xazin Feb 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xazin commented Feb 14, 2023

Uh oh!

Xazin commented Feb 28, 2023

Uh oh!

Xazin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xazin commented Mar 16, 2023

Uh oh!

wizlif left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Xazin Feb 12, 2023 •

edited

Loading

Xazin Mar 18, 2023 •

edited

Loading