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

[#553] Define BaseScreen composable for all screens #533

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

luongvo
Copy link
Member

@luongvo luongvo commented Oct 6, 2023

Closes #553

What happened 👀

  • Create a BaseScreen composable for all screens, as we did with BaseActivity or BaseFragment.
  • Add accompanist-systemuicontroller to demo the status bar color changing between screens.

Insight 📝

The BaseScreen will contain base parameters to request & base logic in the body to execute for all screens.

@Composable
fun BaseScreen(
    // TODO Base parameters to request on all screens here
    content: @Composable () -> Unit,
) {
    // TODO Base logic for all screens here

    content()
}

Proof Of Work 📹

Screen_recording_20231031_180930.mp4

@luongvo luongvo self-assigned this Oct 6, 2023
@luongvo luongvo temporarily deployed to template-compose October 6, 2023 04:09 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

3 Warnings
⚠️ Uh oh! BaseScreen.kt is under 95% coverage!
⚠️ Uh oh! HomeScreen.kt is under 95% coverage!
⚠️ Uh oh! Your project is under 80% coverage!

Kover report for template-compose:

🧛 Template - Compose Unit Tests Code Coverage: 61.54%

Coverage of Modified Files:

File Coverage
BaseScreen.kt 50.77%
HomeScreen.kt 65.00%

Modified Files Not Found In Coverage Report:

ComposableUtil.kt
SecondScreen.kt
ThirdScreen.kt
Versions.kt
build.gradle.kts
colors.xml
styles.xml

Codebase cunningly covered by count Shroud 🧛

Generated by 🚫 Danger

@luongvo luongvo temporarily deployed to template-compose October 6, 2023 04:43 — with GitHub Actions Inactive
@luongvo luongvo force-pushed the chore/improve-the-base-navigation branch 2 times, most recently from 9af6f4c to 790795a Compare October 31, 2023 11:02
@luongvo luongvo force-pushed the feature/define-basescreen-component branch from 8da6706 to 2edce96 Compare October 31, 2023 11:04
@luongvo luongvo changed the title [POC] Define BaseScreen composable for all screens [#553] Define BaseScreen composable for all screens Oct 31, 2023
@luongvo luongvo added this to the 3.26.0 milestone Oct 31, 2023
@luongvo luongvo linked an issue Oct 31, 2023 that may be closed by this pull request
@luongvo luongvo marked this pull request as ready for review October 31, 2023 11:11
@luongvo luongvo force-pushed the chore/improve-the-base-navigation branch from 790795a to ad26c24 Compare October 31, 2023 11:13
@luongvo luongvo force-pushed the feature/define-basescreen-component branch from 3890ae3 to 6034a33 Compare October 31, 2023 11:14
@luongvo luongvo temporarily deployed to template-compose October 31, 2023 11:14 — with GitHub Actions Inactive
@luongvo luongvo force-pushed the feature/define-basescreen-component branch from 6034a33 to c5fe65f Compare November 22, 2023 02:46
@kaungkhantsoe
Copy link
Contributor

Just one concern. Isn't accompanist-systemuicontroller deprecated? Is it a good idea to keep using it for the project?

@ryan-conway ryan-conway modified the milestones: 3.26.0, 3.27.0 Dec 1, 2023
Base automatically changed from chore/improve-the-base-navigation to develop December 6, 2023 09:55
@chornerman
Copy link
Member

@luongvo Please resolve the conflict 🙏🏼

@ryan-conway ryan-conway modified the milestones: 3.27.0, 3.29.0 Apr 1, 2024
@luongvo luongvo force-pushed the feature/define-basescreen-component branch from c5fe65f to a43d6c0 Compare May 27, 2024 17:45
@luongvo luongvo temporarily deployed to template-compose May 27, 2024 17:45 — with GitHub Actions Inactive
@luongvo
Copy link
Member Author

luongvo commented May 27, 2024

Just one concern. Isn't accompanist-systemuicontroller deprecated? Is it a good idea to keep using it for the project?

@kaungkhantsoe, I think we can keep the current approach. It's a deprecated approach, but I added it to the sample project so we can refactor it later. Not a big deal, right?
@chornerman rebased ✅

@luongvo
Copy link
Member Author

luongvo commented Jun 4, 2024

@ryan-conway This one could be processed now. It's also fine if you want to get more reviews from @nimblehq/chapter-android 🏃

@ryan-conway ryan-conway merged commit 1cc164b into develop Jun 5, 2024
4 checks passed
@ryan-conway ryan-conway deleted the feature/define-basescreen-component branch June 5, 2024 09:36
@github-actions github-actions bot mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define BaseScreen composable for all screens
5 participants