Skip to content

Added device compatibility mode snippets. #492

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

Merged
merged 28 commits into from
Apr 18, 2025
Merged

Conversation

JonEckenrode
Copy link
Contributor

No description provided.

Copy link

snippet-bot bot commented Apr 8, 2025

Here is the summary of changes.

You are about to add 4 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@JonEckenrode JonEckenrode requested a review from alexvanyo April 8, 2025 19:04
@@ -62,6 +62,7 @@ wear = "1.3.0"
wearComposeFoundation = "1.4.1"
wearComposeMaterial = "1.4.1"
wearToolingPreview = "1.0.0"
junitVersion = "1.2.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - this should be androidx-test-junit since this is a version for androidx.test extensions of junit, not junit itself which there's also a version for defined above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

@@ -46,7 +46,7 @@ android {

}
dependencies {
val composeBom = platform(libs.androidx.compose.bom)
val composeBom = platform(libs.androidx.compose.bom)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - this formatting change is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored. One of the formatters did that.--I think Android Studio.

import org.junit.Rule;
import static org.junit.Assert.assertFalse;

public class DeviceCompatibilityModeTestJavaSnippets extends AppCompatActivity {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - I don't think any of these test classes need to subclass Activity, they can just be plain classes that don't extend anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct. I got too used to doing this for other snippets. Removed.

// [START android_device_compatibility_mode_assert_isLetterboxed_java]
@Rule
public ActivityScenarioRule<MainActivity> rule = new ActivityScenarioRule<>(MainActivity.class);

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should ba annotated with @Test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Good review, Alex. Thanks.

@JonEckenrode
Copy link
Contributor Author

JonEckenrode commented Apr 12, 2025

A lot of commits. Edited on a plane without the help of Android Studio.

@JonEckenrode JonEckenrode requested a review from alexvanyo April 12, 2025 02:35
import androidx.appcompat.app.AppCompatActivity
import androidx.window.layout.WindowMetricsCalculator

class DeviceCompatibilityModeKotlinSnippets : AppCompatActivity() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - this doesn't need to extend AppCompatActivity either

@JonEckenrode JonEckenrode merged commit d68fdfb into main Apr 18, 2025
5 checks passed
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.

3 participants