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

Fix react navigation issues with IOUModal #1771

Merged
merged 3 commits into from
Mar 15, 2021
Merged

Conversation

Julesssss
Copy link
Contributor

Details

Fixes a couple of issues related to the IOUModal pages and the recent React-native-navigation implementation:

  • Reverted to the onPress prop to fix the close button
  • Wrapped Modal pages in <ScreenWrapper>, fixing the status bar margin on mobile and aligning with other Modals
  • DOES NOT FIX the issue where navigating directly to localhost/iou/request shows a grey background, as this doesn't occur when users use the navigation routing (should investigate this later)

Fixed Issues

Fixes #1769

Tests

Save the following diff as diff.txt and apply with git apply diff.txt (make sure you include the empty new line!)

diff --git a/src/components/CreateMenu.js b/src/components/CreateMenu.js
index 7c7afe3a..e37c842c 100644
--- a/src/components/CreateMenu.js
+++ b/src/components/CreateMenu.js
@@ -52,13 +52,13 @@ class CreateMenu extends PureComponent {
         const menuItemData = [
             {
                 icon: ChatBubble,
-                text: 'New Chat',
-                onPress: () => this.setOnModalHide(() => Navigation.navigate(ROUTES.NEW_CHAT)),
+                text: 'New Request',
+                onPress: () => this.setOnModalHide(() => Navigation.navigate(ROUTES.IOU_REQUEST)),
             },
             {
                 icon: Users,
-                text: 'New Group',
-                onPress: () => this.setOnModalHide(() => Navigation.navigate(ROUTES.NEW_GROUP)),
+                text: 'New Split',
+                onPress: () => this.setOnModalHide(() => Navigation.navigate(ROUTES.IOU_BILL)),
             },
         ].map(item => ({
             ...item,

Confirm close button now works

  • Press the + (FAB) and select Request Money OR Split Bill
  • Attempt to close the modal with the X button
  • Modal should close
  • Reopen, click next to navigate through the steps
  • Attempt to close the modal with the X button
  • Modal should close

Confirm header does not overlap the status bar

  • Press the + (FAB) and select Request Money OR Split Bill
  • Confirm that the status bar is not overlapped (margin should be similar to the Profile page header)

Example of existing problem:
Screenshot 2021-03-15 at 12 47 10

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

ioustatusweb

Mobile Web

ioustatusmweb

Desktop

ioustatusdesktop

iOS

ioustatusios

Android

ioustatusandroid

@Julesssss Julesssss requested a review from a team March 15, 2021 13:04
@Julesssss Julesssss self-assigned this Mar 15, 2021
@botify botify requested review from flodnv and removed request for a team March 15, 2021 13:04
Copy link
Contributor

@flodnv flodnv left a comment

Choose a reason for hiding this comment

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

Just 1 clarifying question for my knowledge 😁

src/pages/iou/IOURequestPage.js Outdated Show resolved Hide resolved
src/pages/iou/IOURequestPage.js Show resolved Hide resolved
@Julesssss Julesssss requested a review from a team as a code owner March 15, 2021 13:24
@botify botify requested review from flodnv and removed request for a team March 15, 2021 13:25
@Julesssss
Copy link
Contributor Author

@flodnv Fixed JSlint and move the disabled check to line 8 👍

@flodnv flodnv merged commit a7179a6 into master Mar 15, 2021
@flodnv flodnv deleted the jules-fixIOUModalNavigation branch March 15, 2021 13:50
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IOUModal View Issue
2 participants