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

[feature] Implement License Screen #883

Merged
merged 15 commits into from
Aug 26, 2023

Conversation

fumiya-kume
Copy link
Contributor

@fumiya-kume fumiya-kume commented Aug 22, 2023

Issue

Overview (Required)

  • Add plugin to apply the OSS Licenses Gradle Plugin
  • Add navigation logic for the license screen which generated the plugin.
  • Add theme config for the generated Activities by the plugin.

Title of the screen

I created the issue #938

TODO

  • Solve issue that refer the LicensePlugin with Gradle Version Catalog
  • Solve issue that I'd like to not changing the theme...

Links

Screenshot (Optional if screenshot test is present or unrelated to UI)

Before After

Movie of License screen behavior

Before After
N/A
license-qa.webm

@fumiya-kume fumiya-kume force-pushed the kuu/add-license-screen branch from e7dda6e to 4a0b1c0 Compare August 22, 2023 14:19
@fumiya-kume fumiya-kume changed the title Kuu/add license screen [feature] Add license screen Aug 22, 2023
@fumiya-kume fumiya-kume changed the title [feature] Add license screen [feature] Implement License Screen Aug 22, 2023
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>

<style name="Theme.KaigiApp" parent="android:Theme.Material.Light.NoActionBar">
<style name="Theme.KaigiApp" parent="Theme.AppCompat.Light.NoActionBar">
Copy link
Member

@takahirom takahirom Aug 22, 2023

Choose a reason for hiding this comment

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

🙏
(I thought it is ok to have this theme 👀 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me check with revert ver 🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have to change the theme for entire the app.

I can just update the theme for the Activities which generate by the plugin.

Comment on lines 128 to 132

# etc

ossLicenses = { module = "com.google.android.gms:play-services-oss-licenses", version.ref = "ossLicenses" }
ossLicensesPlugin = { module = "com.google.android.gms:oss-licenses-plugin", version.ref = "ossLicensesPlugin" }
Copy link
Member

Choose a reason for hiding this comment

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

How about placing these dependencies below the dependencies for tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's make sense. thank you for suggestion!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved to below of tests

build.gradle.kts Outdated
Comment on lines 15 to 17
dependencies {
classpath(libs.ossLicensesPlugin)
}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for recommendations!
I'm not family with this, so much helpful 🙇

Copy link
Contributor Author

@fumiya-kume fumiya-kume Aug 24, 2023

Choose a reason for hiding this comment

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

I created the Plugin for OSS license plugin.

also It applied.

@github-actions
Copy link

github-actions bot commented Aug 22, 2023

Test Results

190 tests   190 ✔️  7m 12s ⏱️
  11 suites      0 💤
  11 files        0

Results for commit c06afc0.

♻️ This comment has been updated with latest results.

@github-actions github-actions bot temporarily deployed to deploygate-distribution August 22, 2023 16:29 Inactive
@DroidKaigi DroidKaigi deleted a comment from github-actions bot Aug 22, 2023
@github-actions github-actions bot temporarily deployed to deploygate-distribution August 22, 2023 23:30 Inactive
@github-actions
Copy link

Hi @fumiya-kume! Codes seem to be unformatted. To resolve this issue, please run ./gradlew detekt --auto-correct and fix the results of ./gradlew lintDebug.. Thank you for your contribution.

@github-actions github-actions bot temporarily deployed to deploygate-distribution August 23, 2023 15:49 Inactive
@@ -28,6 +28,14 @@
</intent-filter>
</activity>

<activity
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 Specify the AppCompat theme for the OSS license plugin's screen

@fumiya-kume fumiya-kume marked this pull request as ready for review August 24, 2023 11:01
@fumiya-kume fumiya-kume requested a review from a team as a code owner August 24, 2023 11:01
@github-actions github-actions bot temporarily deployed to deploygate-distribution August 24, 2023 12:27 Inactive
@@ -98,6 +98,10 @@ gradlePlugin {
id = "droidkaigi.primitive.detekt"
implementationClass = "io.github.droidkaigi.confsched2023.primitive.DetektPlugin"
}
register("oss-licenses") {
id = "droidkaigi.primitive.osslicenses"
Copy link
Member

@takahirom takahirom Aug 25, 2023

Choose a reason for hiding this comment

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

Thanks! We might have to use droidkaigi.primitive.*android*.osslicenses because the license list plugin is based on AGP plugin.

@takahirom
Copy link
Member

Almost LGTM! Thanks

Copy link
Contributor

@momomomo111 momomomo111 left a comment

Choose a reason for hiding this comment

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

After verifying, the license displayed correctly in the release environment, but it did not function properly in the debug environment.
If this is the intended behavior, then I believe it's fine.
Thank you for contributing!!👍

@fumiya-kume
Copy link
Contributor Author

After verifying, the license displayed correctly in the release environment, but it did not function properly in the debug environment.

If this is the intended behavior, then I believe it's fine.

Thank you for contributing!!👍

I don't have much experience with the plug-in, then I'd hear other person's opinion if there.

I other hand, I confirmed that the behavior already.

@tarumzu
Copy link
Contributor

tarumzu commented Aug 25, 2023

@momomomo111 oss-licenses-plugin-v0.10.5 and later are only visible in release builds.

google/play-services-plugins@138a15d (#216)

@github-actions github-actions bot temporarily deployed to deploygate-distribution August 26, 2023 09:50 Inactive
@momomomo111 momomomo111 merged commit 10c25bd into DroidKaigi:main Aug 26, 2023
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.

Implement License Screen
4 participants