Skip to content

Comments

Modify app#2

Open
romanoprid wants to merge 7 commits intomainfrom
modify_app
Open

Modify app#2
romanoprid wants to merge 7 commits intomainfrom
modify_app

Conversation

@romanoprid
Copy link
Owner

Here you can see the previous application which was remade and some new functionality was added. Here are two screens:
the first one is a list of items that you can add to the cart (the second screen) by clicking on the icon on the right side of an element. Also, you can delete items from the cart using the round icon on the right side of the element, see the total price of the added elements and click on the "buy" button which functional maybe will be added in the future.

  1. Catalog(with added items)

image

  1. Cart (with added items and total price)

image

  1. Cart (with items that were not deleted and new total price)

image

@romanoprid romanoprid self-assigned this Sep 23, 2021
@romanoprid romanoprid requested review from JohnSteck9 and user-nazar and removed request for JohnSteck9 September 25, 2021 07:50
Copy link
Collaborator

@xor3r xor3r left a comment

Choose a reason for hiding this comment

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

No major problems, can be merged.

const SliverToBoxAdapter(child: SizedBox(height: 12)),
SliverList(
delegate: SliverChildBuilderDelegate(
(context, index) => _MyListItem(index)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe consider renaming _MyListItem to something more self-explanatory.

Copy link
Owner Author

@romanoprid romanoprid Sep 27, 2021

Choose a reason for hiding this comment

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

Thanks. It will be edited soon.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Try to name variables of the small size for readability

Copy link
Collaborator

@maksymbudzin maksymbudzin left a comment

Choose a reason for hiding this comment

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

This lab was chacked, everything is okay, please marge!

Copy link

@igor-leshkevych igor-leshkevych left a comment

Choose a reason for hiding this comment

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

@romanoprid

+5/5 hello world
+3/3 custom feature + PR

Resolve conflicts before merge.

defaultConfig {
// TODO: Specify your own unique Application ID (https://developer.android.com/studio/build/application-id.html).
applicationId "com.example.hometask_flutter_v2"
applicationId "com.example.hometask_v_3"

Choose a reason for hiding this comment

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

version (v2, v3) shouldn't be a part of your app id (as your version will change regularly, while your package ID won't ever change after app is published)

@igor-leshkevych
Copy link

@JohnSteck9
+2/2 code review, ideally though I'd like to see some comments, not just pressed "Approve" button (not because you need to do that even if you have nothing to say, but for learning purposes in this particular lab task)

@user-nazar
Copy link
Collaborator

Good work

Copy link

@maximlev0-9 maximlev0-9 left a comment

Choose a reason for hiding this comment

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

Overall it looks great!

@@ -1,18 +1,3 @@
# FlutterCourse
It is the repsitory from mobile development subject

Choose a reason for hiding this comment

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

Typo: "repository"

@igor-leshkevych
Copy link

@maximlev0-9
+2/2 review

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.

7 participants