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.

Sign up for GitHub

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

Jump to bottom

Create calendar page layout and set up navigation #25

Merged
merged 40 commits into from
Dec 5, 2022
Merged

Create calendar page layout and set up navigation #25

merged 40 commits into from
Dec 5, 2022

Conversation

michaelng0107
Copy link
Contributor

@michaelng0107 michaelng0107 commented Nov 28, 2022 •
edited
Loading

Closes #3 and closes #4.

Added common bottom navigation bar to CommonComponents.kt
Added Calendar Tab layout to CalendarScreen.kt
Created NavigationGraph.kt to organize routes.

Notes:

@michaelng0107 michaelng0107 added the CALENDAR This goes to the calendar page label Nov 28, 2022
@michaelng0107
added dependencies and tests
d6271a8
@michaelng0107
Added activity to manifest - temp fix so tests run
e148cd1 
Was seeing the following issues when I run tests in NavigationTest:
- android.content.ActivityNotFoundException: Unable to find explicit activity class {com.example.theperiodpurse.test/androidx.test.core.app.InstrumentationActivityInvoker$EmptyActivity}; have you declared this activity in your AndroidManifest.xml, or does your intent not match its declared <intent-filter>?
- android.content.ActivityNotFoundException: Unable to find explicit activity class {com.example.theperiodpurse.test/androidx.test.core.app.InstrumentationActivityInvoker$BootstrapActivity}; have you declared this activity in your AndroidManifest.xml, or does your intent not match its declared <intent-filter>?

Referred to android/android-test#196 for the fix.
Looks like this is fixed in androidx.test:core:1.5.0-alpha02.
@michaelng0107
removed changes to manifest
03b6c1a
@michaelng0107
added androidTestResultsUserPreferences to gitignore
b3a4cd8
michaelng0107
michaelng0107 commented Nov 29, 2022
View reviewed changes
app/src/androidTest/java/com/example/theperiodpurse/NavigationTest.kt Outdated
@Test
fun appNavHost_clickCalendarFABOnSettingsScreen_navigatesToCalendarScreen() {
navigateToSettingsScreen()
composeTestRule.onNodeWithContentDescription("fab")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There might be a better way of getting the FAB node. It doesn't have text on it so I had to use something like contentDescription. We could look into test tags/making contentDescriptions more standardized

@michaelng0107
Copy link
Contributor Author

michaelng0107 commented Nov 29, 2022 •
edited
Loading

Don't think the AndroidTests (where the navigation tests are) are running on CI since we're only running ./gradlew test. Should we also add ./gradlew connectedAndroidTest to run instrumental tests?

https://developer.android.com/studio/test/command-line#run-tests-with-gradle

@leowrites
Copy link
Member

leowrites commented Nov 29, 2022 •
edited
Loading

@michaelng0107

Don't think the AndroidTests (where the navigation tests are) are running on CI since we're only running ./gradlew test. Should we also add ./gradlew connectedAndroidTest to run instrumental tests?

https://developer.android.com/studio/test/command-line#run-tests-with-gradle

Yes, you can add that to the job in the workflow

@madelahn madelahn self-assigned this Nov 29, 2022
@michaelng0107
Copy link
Contributor Author

Something we have to think about is whether its okay to change the CI VM from ubuntu-latest to macOS-latest, and if a 20 minute github action is acceptable.

@leowrites
Copy link
Member

leowrites commented Dec 1, 2022 •
edited
Loading

@michaelng0107

Something we have to think about is whether its okay to change the CI VM from ubuntu-latest to macOS-latest, and if a 20 minute github action is acceptable.

In https://github.com/marketplace/actions/android-emulator-runner, there is a section on reducing startup time through AVD snapshot caching. Maybe that can reduce the 20 minutes?

@michaelng0107 michaelng0107 marked this pull request as ready for review December 2, 2022 16:38
leowrites
leowrites approved these changes Dec 5, 2022
View reviewed changes
Copy link
Member

@leowrites leowrites left a comment

Choose a reason for hiding this comment

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

Great work, the code is very clean 👍

@leowrites leowrites requested a review from elenawzy December 5, 2022 01:42
@madelahn
#3 Created the Calendar Layout (#33)
18909c4 
* Added basic calendar setup to the screen

* Changed the minSDK in Project Configs

* Updated Visuals to look like original app

* Added Click Functionality
@madelahn
Revert "#3 Created the Calendar Layout (#33)" (#38)
0ca3e86 
This reverts commit 18909c4.
@leowrites leowrites merged commit 4a8f501 into develop Dec 5, 2022
@michaelng0107 michaelng0107 deleted the calendar-nav branch December 6, 2022 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leowrites leowrites leowrites approved these changes

@elenawzy elenawzy elenawzy approved these changes

Assignees

@michaelng0107 michaelng0107

@PierreLessard PierreLessard

@madelahn madelahn

Labels
CALENDAR This goes to the calendar page
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement the navigation between different pages Create the calendar page layout
5 participants
@michaelng0107 @leowrites @elenawzy @PierreLessard @madelahn