Create IOU Amount step #1768
Create IOU Amount step #1768
Conversation
There was a problem hiding this comment.
I'm a newbie here so please forgive me if my questions have obvious answers 😊
I'll be leaning on @Julesssss for a thorough review
There was a problem hiding this comment.
Looking really good! Added some comments and questions.
FYI, there's a problem with react-native-navigation where the first back button press isn't recognised, but no need to worry about fixing that on this PR.
Also, would it be possible to make the BNP input focused by default on Web/Desktop?
|
Instead of having a gray "Next" button that turns green when there is an amount entered, I think it might be better to just have a disabled state for our green success buttons. Perhaps it's the same button styles but it's just at 50% opacity or something. Also - why does the screen show "USD" instead of just a |
I updated Next button view when disabled. I created new style for disabled green success button
In the issue, I was told that it is OK to use currency code for now.
|
I tried to use autoFocus prop on TextInput, but it doesn't work as expected when chat views are on the background. It works as expected for /iou/request where background is solid grey. Do you know what would be the reason here? |
Ah, good find. I recall that we built logic for autofocusing on the chat text input, but don't know what exactly we need to change. I have asked internally to see if anyone has an idea.
This is blocked for now, will be completed with a separate follow up issue. |
|
Actually @tugbadogan, I have one suggestion for the input issue. Have you tried using |
I tried using |
There was a problem hiding this comment.
I tried using
TextInputFocusableand it is working if request is navigated within page but it doesn't focus when page is loaded with direct URL. Same problem exists for other focusable text inputs like search.
Ah that's fantastic, I'd say that directly loading the URL is an edge case and that can be fixed as a separate bug.
The final changes to make as far as I can see are:
- Switch to the hidden input trick you mentioned, that will allow us to keep the amount Text horizontally aligned
- Prevent the mobile keyboard from being displayed (screenshots below)
I implemented this feature and will update PR.
I couldn't reproduce this issue as there is no text input. |
There was a problem hiding this comment.
I'm still seeing the Keyboard displayed on a physical device when the IOUAmount page is displayed. It isn't always consistent but occurs most of the time, are you able to reproduce this? I can see it on both Android and iOS physical devices.
iouAmountKeyboard.mp4
|
Hi @tugbadogan, I've just noticed that the same issue occurs prior to your changes, so no need to view the keyboard issue as a blocker, I'll raise a separate issue for that. |
- Create different view for Mobile and Web/Desktop
- Show title based on current route ('Split Bill' or 'Request Money'
- Accept decimal number up to 6 digits and 2 decimal digit
- Create different view for Mobile and Web/Desktop
- Show title based on current route ('Split Bill' or 'Request Money'
- Accept decimal number up to 6 digits and 2 decimal digit
|
@flodnv Fixed merge conflicts and updated the PR. |
There was a problem hiding this comment.
@tugbadogan please avoid force pushing in the future as this has the side effect of requiring a full re-review from reviewers. We'll add a guideline to that effect in the repo 🙇
@Julesssss all yours.
| textAlign="left" | ||
| /> | ||
| <Text | ||
| style={[styles.iouAmountText, styles.invisible, {left: 100000}]} |
There was a problem hiding this comment.
Please don't use inline styles like {left: 100000} directly in the style tag.
Also, why 100000? It looks maybe like a magic number?
@trjExpensify @Julesssss
Details
hasMultipleParticipantsprop ('Split Bill' or 'Request Money')Fixed Issues
Fixes #1691
Tests
Tested On
Screenshots
Web
1Screen.Recording.2021-03-14.at.15.46.33.mov
Mobile Web
Screen.Recording.2021-03-14.at.15.49.22.mov
Desktop
iOS
Android