Skip to content

Conversation

@li-advait
Copy link
Contributor

@li-advait li-advait commented Jun 12, 2025

Add support to capture screenshots for modals which are linked to window root view rather than activity root view.

Requires adding more permissions for capturing screenshots using MediaProjectionManager API:
MEDIA_PROJECTION_SERVICE

Before adding support for this API:
Screenshot 2025-06-20 at 2 14 53 PM

After adding support for this API:
Screenshot 2025-06-20 at 2 13 29 PM

@li-advait li-advait requested a review from Copilot June 20, 2025 21:19
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds MediaProjection-based screenshot capture to include overlays in modal dialogs.

  • Enables toggling between default and MediaProjection screenshot methods in Shaky.
  • Introduces ScreenCaptureService and ScreenCaptureManager to handle permission, foreground service, and capture flow.
  • Updates the sample app with a bottom sheet demo, styles, permissions, and Gradle dependencies.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
shaky/src/main/java/com/linkedin/android/shaky/Shaky.java Added fields, methods, and flow logic to support MediaProjection.
shaky/src/main/java/com/linkedin/android/shaky/ScreenCaptureService.java New foreground service for MediaProjection on Android 10+.
shaky/src/main/java/com/linkedin/android/shaky/ScreenCaptureManager.java New manager to request permission and perform screen capture.
shaky/src/main/AndroidManifest.xml Added permissions and service declaration for MediaProjection.
shaky-sample/src/main/res/values/styles.xml Defined custom bottom sheet dialog styles.
shaky-sample/src/main/res/values/strings.xml Added string resource for opening the bottom sheet.
shaky-sample/src/main/res/layout/activity_demo.xml Added a button to launch the bottom sheet.
shaky-sample/src/main/java/com/linkedin/android/shaky/app/ShakyDemo.java Switched to FragmentActivity, wired up bottom sheet and result handling.
shaky-sample/src/main/java/com/linkedin/android/shaky/app/SampleBottomSheetDialog.java New bottom sheet dialog fragment with styling and layout.
shaky-sample/src/main/AndroidManifest.xml Declared needed permissions for MediaProjection in the sample.
shaky-sample/build.gradle Added Material Components dependency.
Comments suppressed due to low confidence (1)

shaky/src/main/java/com/linkedin/android/shaky/ScreenCaptureManager.java:1

  • New screen capture functionality (permission flow, virtual display, image processing) lacks unit or instrumentation tests. Consider adding tests to cover success and failure paths.
package com.linkedin.android.shaky;

Comment on lines +66 to +67
private CollectDataTask.Callback pendingDataCollectionCallback = null;

Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

The field 'pendingDataCollectionCallback' is declared but never used. Consider removing it or implementing its intended usage to avoid dead code.

Suggested change
private CollectDataTask.Callback pendingDataCollectionCallback = null;

Copilot uses AI. Check for mistakes.
<item name="shapeAppearanceOverlay">@style/CustomShapeAppearanceBottomSheetDialog</item>
</style>

<style name="CustomShapeAppearanceBottomSheetDialog" parent="">
Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

The 'parent' attribute is empty, which will break style inheritance at runtime. Specify a valid parent style (e.g., a ShapeAppearance material component).

Suggested change
<style name="CustomShapeAppearanceBottomSheetDialog" parent="">
<style name="CustomShapeAppearanceBottomSheetDialog" parent="ShapeAppearance.MaterialComponents">

Copilot uses AI. Check for mistakes.
Comment on lines +71 to +78
view.getViewTreeObserver().addOnGlobalLayoutListener(() -> {
// Get the parent view of the bottom sheet
View bottomSheet = (View) view.getParent();
// Set up the bottom sheet behavior
BottomSheetBehavior<View> behavior = BottomSheetBehavior.from(bottomSheet);
// Set the height to 50% of screen
behavior.setPeekHeight(getResources().getDisplayMetrics().heightPixels / 2);
behavior.setState(BottomSheetBehavior.STATE_EXPANDED);
Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

[nitpick] You register an OnGlobalLayoutListener without removing it after the first callback, which can lead to repeated invocations or memory leaks. Remove the listener once layout is complete.

Suggested change
view.getViewTreeObserver().addOnGlobalLayoutListener(() -> {
// Get the parent view of the bottom sheet
View bottomSheet = (View) view.getParent();
// Set up the bottom sheet behavior
BottomSheetBehavior<View> behavior = BottomSheetBehavior.from(bottomSheet);
// Set the height to 50% of screen
behavior.setPeekHeight(getResources().getDisplayMetrics().heightPixels / 2);
behavior.setState(BottomSheetBehavior.STATE_EXPANDED);
view.getViewTreeObserver().addOnGlobalLayoutListener(new ViewTreeObserver.OnGlobalLayoutListener() {
@Override
public void onGlobalLayout() {
// Get the parent view of the bottom sheet
View bottomSheet = (View) view.getParent();
// Set up the bottom sheet behavior
BottomSheetBehavior<View> behavior = BottomSheetBehavior.from(bottomSheet);
// Set the height to 50% of screen
behavior.setPeekHeight(getResources().getDisplayMetrics().heightPixels / 2);
behavior.setState(BottomSheetBehavior.STATE_EXPANDED);
// Remove the listener to prevent repeated invocations
view.getViewTreeObserver().removeOnGlobalLayoutListener(this);
}

Copilot uses AI. Check for mistakes.
private Notification createNotification() {
NotificationCompat.Builder builder = new NotificationCompat.Builder(this, NOTIFICATION_CHANNEL_ID)
.setContentTitle("Capturing Screenshot")
.setContentText("Processing your feedback...")
Copy link

Copilot AI Jun 20, 2025

Choose a reason for hiding this comment

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

[nitpick] Hardcoded notification text should be moved to string resources for localization support.

Copilot uses AI. Check for mistakes.
@li-advait li-advait merged commit 7f90eb2 into linkedin:main Jul 7, 2025
2 checks passed
@li-advait li-advait deleted the li-advait/support_screenshots_for_modals branch July 7, 2025 18:09
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.

2 participants