Skip to content

Zahra Alnamer- Crushed It 🥇 #3

Open
ZahraHN-03 wants to merge 3 commits intoProgramming-Club-IAU:masterfrom
ZahraHN-03:feature
Open

Zahra Alnamer- Crushed It 🥇 #3
ZahraHN-03 wants to merge 3 commits intoProgramming-Club-IAU:masterfrom
ZahraHN-03:feature

Conversation

@ZahraHN-03
Copy link

No description provided.

Copy link

@Radwan-Albahrani Radwan-Albahrani left a comment

Choose a reason for hiding this comment

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

Overall Impressive work, Love the design, and I truly appreciate the amount of time you've put into it. Really Crushed this Task!

Below are some recommendations to improve code readibility and fix some layout mistakes. Overall, Very impressed with what you've accomplished.

You can mark the task as complete!!

lib/main.dart Outdated
),
),
return MaterialApp(
title: 'Wether App',

Choose a reason for hiding this comment

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

misspelled

Suggested change
title: 'Wether App',
title: 'Weather App',

),
return MaterialApp(
title: 'Wether App',
initialRoute: LoginPage.route,

Choose a reason for hiding this comment

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

Since you are getting the initial route as a parameter in the constructor, why not use it?

Suggested change
initialRoute: LoginPage.route,
initialRoute: initialRoute,

Comment on lines +23 to +26
routes: {
LoginPage.route: (context) => const LoginPage(),
HomeScreen.route: (context) => const HomeScreen(),
},

Choose a reason for hiding this comment

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

same here. you are getting routes from the constructor

class MainApp extends StatelessWidget {
const MainApp({super.key});
class MyApp extends StatelessWidget {
const MyApp({super.key, required initialRoute, required routes});

Choose a reason for hiding this comment

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

To access them, you need variables for them

Suggested change
const MyApp({super.key, required initialRoute, required routes});
const MyApp({super.key, required this.initialRoute, required this.routes});
final String initialRoute;
final Map<String, WidgetBuilder> routes;

),
child: ElevatedButton(
onPressed: () {
Navigator.of(context).pushNamed(HomeScreen.route);

Choose a reason for hiding this comment

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

You were on the right track but stopped. You made a navigatePage function but didn't complete it

Suggested change
Navigator.of(context).pushNamed(HomeScreen.route);
navigateNextPage();


class _HeaderWidgetState extends State<HeaderWidget> {
String city = "";
String date = DateFormat('EEE, MMM d').format(DateTime.now());

Choose a reason for hiding this comment

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

Absolutely great idea displaying the date

@@ -0,0 +1,17 @@
import 'package:flutter/material.dart';

class SnowFlakes extends StatelessWidget {

Choose a reason for hiding this comment

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

Usually filename should match widget name

HomeScreen.route: (context) => const HomeScreen(),
},
home: const LoginPage(),
);

Choose a reason for hiding this comment

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

you forgot to remove debug banner

Suggested change
);
debugShowCheckedModeBanner: false,
);

import 'package:get/get.dart';
import 'package:geolocator/geolocator.dart';

class GlobalController extends GetxController {

Choose a reason for hiding this comment

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

Fantastic work on the location controller. I would recommend documenting it.

HourlyDataWidget(),
DailyData(),
],
))),

Choose a reason for hiding this comment

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

commas

Suggested change
))),
),),),

@ZahraHN-03 ZahraHN-03 changed the title Zahra Alnamer- Crashed It 🥇 Zahra Alnamer- Crauhed It 🥇 Oct 24, 2023
@ZahraHN-03 ZahraHN-03 changed the title Zahra Alnamer- Crauhed It 🥇 Zahra Alnamer- Crushed It 🥇 Oct 24, 2023
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