Skip to content
This repository was archived by the owner on Aug 21, 2025. It is now read-only.

Implement Expanded QR#97

Merged
hamorillo merged 5 commits intotrunkfrom
hamorillo/GRA-580
Aug 1, 2025
Merged

Implement Expanded QR#97
hamorillo merged 5 commits intotrunkfrom
hamorillo/GRA-580

Conversation

@hamorillo
Copy link

@hamorillo hamorillo commented Jul 31, 2025

Description

This PR implements the expanded QR code. In the Share Screen, we are adding a new option that allows users to display the QR Code with the contact information in a full-screen presentation.

Kapture.2025-08-01.at.09.51.00.mp4

Testing Steps

  • Launch the app and switch to the share screen
  • Click on the expand icon
  • Verify the QrCode is presented in full screen (system bars should disappear)
  • Tap the close button and verify you navigate to the Share screen (system bars should appear)
  • Click on the expand icon
  • Verify the QrCode is presented in full screen (system bars should disappear)
  • Press the back button and verify you navigate to the Share screen (system bars should appear)

@hamorillo hamorillo added the enhancement New feature or request label Jul 31, 2025
@hamorillo hamorillo force-pushed the hamorillo/GRA-580 branch from 3e738f1 to c36bbb5 Compare July 31, 2025 23:12
@hamorillo hamorillo requested a review from Copilot August 1, 2025 07:51
Copy link
Contributor

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 implements an expanded QR code feature that allows users to view a QR code in full screen mode with hidden system bars for better visibility when sharing contact information.

Key changes include:

  • Added expandable QR code functionality with fullscreen display
  • Implemented bottom bar visibility control for better user experience
  • Extracted shared QR code rendering logic into a reusable component

Reviewed Changes

Copilot reviewed 19 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ExpandedQrCode.kt New fullscreen QR code component with system bar hiding
QrCode.kt Extracted reusable QR code rendering component
ShareHeader.kt Added expand button and refactored to use extracted QR component
ShareViewModel.kt Added QR expansion state management and bottom bar control
ShareScreen.kt Integrated expanded QR display with animation
HomeViewModel.kt Added bottom bar visibility event handling
Various test files Added comprehensive test coverage for new functionality
Resource files Added new drawable assets and string resources

@hamorillo hamorillo marked this pull request as ready for review August 1, 2025 08:09
@hamorillo hamorillo requested a review from etoledom August 1, 2025 08:09
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Looking great!

Just a few comments (only the first one requires change):

Could we give the close button a bit of padding at the top?

It feels like it's way too close to the top of the screen, and it momentary gets under the status bar.

Maybe something like this (even if it's not exact):
CleanShot 2025-08-01 at 11 55 56@2x

The orientation of the presentation was a bit unexpected

The view presenting from the top left corner felt unexpected 🤔
If the animation could come from straight up-down, or down-up it might feel better.

  • If the current one is common on Android then 👍
  • If not and it's an easy change, let's try 🙏
  • If it's complex and will take time, let's leave it for later 👍

Press the back button and verify you navigate to the Share screen (system bars should appear)

The system back button also gets hidden. The only way I see to dismiss this screen is by the ( x ) button (which I guess is fine).

On iOS we have the swipe down gesture as an alternative dismissal. It feels natural since the presentation is a modal which comes from the bottom up. And it's easier than reaching the high close button.

No need to implement this now, I'd rather move the button down closer to the thumb. So this is a wider discussion for later.

@hamorillo
Copy link
Author

Thanks for the review @etoledom!

About the status bar. I saw the iOS app and decided to keep it, so we are no longer hiding it. Also fixed the padding issue. 👍

The view presenting from the top left corner felt unexpected 🤔
If the animation could come from straight up-down, or down-up it might feel better.

I've updated the animation so it appears from the top of the screen with a fade-in effect and disappears with the opposite animation. Let me know what you think.

The system back button also gets hidden. The only way I see to dismiss this screen is by the ( x ) button (which I guess is fine).

I think this should be fine. Also, you should be able to show the navigation bar by swiping over the bottom area.

Kapture.2025-08-01.at.12.34.24.mp4

Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

I've updated the animation so it appears from the top of the screen with a fade-in effect and disappears with the opposite animation. Let me know what you think.

That's beautiful! 🙏

About the status bar. I saw the iOS app and decided to keep it, so we are no longer hiding it. Also fixed the padding issue. 👍

Perfect! Looks quite natural now 🎉

I think this should be fine. Also, you should be able to show the navigation bar by swiping over the bottom area.

Oh I didn't know that. Now I can corroborate that dismiss with back button works as described 👍


Thank you for the improvements!
🚢

@hamorillo hamorillo merged commit 4e4453c into trunk Aug 1, 2025
10 checks passed
@hamorillo hamorillo deleted the hamorillo/GRA-580 branch August 1, 2025 10:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants