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.

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

Sabrina/market layout #12

Merged
merged 7 commits into from Mar 9, 2020
Merged

Sabrina/market layout #12

merged 7 commits into from Mar 9, 2020

Conversation

@sabrina-li
Copy link
Contributor

sabrina-li commented Mar 4, 2020

Adding the layout xml files for the market views
--this branch needs to be rebased after #9 is merged

@sabrina-li sabrina-li force-pushed the sabrina/marketLayout branch from ced8ea4 to 5fda12a Mar 6, 2020
@sabrina-li sabrina-li assigned sabrina-li and patrick-fs and unassigned sabrina-li Mar 6, 2020
@sabrina-li sabrina-li requested review from HaroldFS and van-fs Mar 6, 2020
@@ -21,13 +21,7 @@ public View onCreateView(@NonNull LayoutInflater inflater,
marketViewModel =

This comment has been minimized.

Copy link
@patrick-fs

patrick-fs Mar 7, 2020

Member

Similar to my comment on the Cart layout PR: this appears instantiated but unused.

This comment has been minimized.

Copy link
@sabrina-li

sabrina-li Mar 9, 2020

Author Contributor

added this ViewModel instance serves as placeholder, since the actual VM logic is dependent on the data and networking classes to provide the data.
Eventually VM will provide the LiveData for the Fragment - to be implemented

This comment has been minimized.

Copy link
@sabrina-li

sabrina-li Mar 9, 2020

Author Contributor

had to remove the original boilerplate code because the TextView that it was looking for is deleted from the layout files.

Copy link
Member

patrick-fs left a comment

LGTM. Added a couple of comments for you tor review.

@sabrina-li sabrina-li merged commit 296391d into master Mar 9, 2020
@sabrina-li sabrina-li deleted the sabrina/marketLayout branch Mar 9, 2020
@sabrina-li sabrina-li mentioned this pull request Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.