-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Fix bugs connected to changed modal logic #47991
Fix bugs connected to changed modal logic #47991
Conversation
Reviewer Checklist
Screenshots/VideosiOS: NativeCleanShot.2024-08-27.at.15.24.48-converted.mp4 |
@zfurtak Uploading an avatar for a user group fails for me.
|
@fedirjh should be fixed 😉 |
@zfurtak Thanks, it's fixed for avatar picker, but it's still exist in the composer's attachment picker
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-08-28.at.16.52.01.mp4 |
@fedirjh it was a similar case 😅 Also fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
@arosiclair I tested all the cases I spotted, also reviewed the previous issues, however I think it will be good if a QA take their time to look closely into this modal, in case I missed something |
Alright lets merge this and see how it fares in QA |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/arosiclair in version: 9.0.28-0 🚀
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.0.28-3 🚀
|
Fixed Issues
$ #46852
Details
This reverts commit 804bd26, reversing changes made to 2026c78.
There were two issues related to this fix: #47896 and #47876. Both issues were observed only on mWeb Safari. The root cause was that opening the image/attachment picker on Safari can't be done from
onSelected
prop, it needs a check and a call from theonItemSelected()
function.PROPOSAL:
I went back to the previous changes that fix the delete receipt problem but also added an additional check in the
PopoverMenu
component (&& !Browser.isSafari
) which fixes the mWeb problems and since theModal.close(() =>
function is specifically required only for iOS, it's not needed in the Safari case.Tests
ThreeDotsMenu
in the upper right corner -> thenDelete receipt
Additional issues resolved:
$ #47896
Upload photo
Photo library, Take Photo, Choose File
$ #47876
+
->Add attachment
Photo library, Take Photo or Video, Choose File
Below I uploaded videos to these 3 cases.
Offline tests
same as above
QA Steps
Besides this issue, I changed code in
Modal
component, it would be great to test modals in different parts of the application.PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Deleting receipt:
android-receipt-1.mov
Changing photo of a group chat:
android-photo.mov
Adding attachment to a message:
android-attach.mov
Android: mWeb Chrome
Deleting receipt:
mweb-android-receipt.mov
Changing photo of a group chat:
mweb-photo.mov
Adding attachment to a message:
mweb-android-attach.mov
iOS: Native
Deleting receipt:ios-receipt-1.mov
Changing photo of a group chat:
ios-photo-1.mov
Adding attachment to a message:
ios-attach-1.mov
iOS: mWeb Safari
Deleting receipt:
mweb-receipt.mov
Changing photo of a group chat:
mweb-photo-1.mov
Adding attachment to a message:
mweb-attach.mov
MacOS: Chrome / Safari
Deleting receipt:
web-receipt.mov
Changing photo of a group chat:
web-photo.mov
Adding attachment to a message:
web-attach.mov
MacOS: Desktop
Deleting receipt:
desktop-receipt.mov
Changing photo of a group chat:
desktop-photo.mov
Adding attachment to a message:
desktop-attach-1.mov