-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Adds: Skeleton classes for Category Detail Screen #16066
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
Conversation
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
You can test the changes on this Pull Request by downloading the APKs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the wait is over and you were finally able to resume this work! 😄 Can't wait to see what's coming next. 🙌
I just left a few nitpicks, feel free to merge once you have taken a look.
viewModel.start(site) | ||
} | ||
|
||
private fun getSite(savedInstanceState: Bundle?): SiteModel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can utilize SelectedSiteRepository
in the VM to get the currently selected site. Feel free to ignore it for now and try it in your implementation PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ashiagr Let's ignore it for now, I hadn't looked at the SelectedSiteRepository
for getting the site model. Thanks for letting me know. I will make that change in the implementation PR.
@ashiagr Thanks for reviewing the PR and giving me suggestions. 🙌 . |
Parent #16058
To test:
As these are only skeleton classes that are not being called in the UI these cannot be tested from the app.
Regression Notes
Potential unintended areas of impact
N/A
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
What automated tests I added (or what prevented me from doing so)
N/A
PR submission checklist:
RELEASE-NOTES.txt
if necessary.