Make button styles consistent in Expensify.cash

michaelhaxhiu commented 3 years ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Expected Result:

Button style attributes should remain consistent across all Expensify.cash platforms (including iOS and Android). More specifically, buttons should behave like so:

They are sticky to the bottom of the page/view
They won't enable unless the user has completed an action (e.g., typed in a text field, selected a dropdown value, etc.)
- When the button is disabled it should have a reduced opacity and we should eliminate mouse/touch events for the disabled states.
They have a white margin to the sides and below the button to prevent the look of overlapping content (see image1 below)

Actual Result:

Button styles are inconsistent right now. Here's a few examples:

image1:

image2:

Buttons to modify:

Settings > Profile Save button
Settings > Profile > Add Phone Number/Add Email > Send Validation button
Settings > Change Password Save button
Settings > Payments Add PayPal Account button
+ FAB > New Group > Select 1 or more people > Create Group button
+ FAB > Request Money > Enter Amount and Press Next > Select an attendee > Enter a comment > Request $XX button.
+ FAB > Split Bill > Enter Amount and Press Next > Select 1 or more people > Next button
+ FAB > Request Money/Split Bills > Press the currency symbol > Search for a currency using the search bar to focus the keyboard > Select a currency > Confirm button.

Platform:

Where is this issue occurring?

Web iOS ✔️ Android ✔️ Desktop App Mobile Web

Version Number: v1.0.50-2 Expensify/Expensify Issue URL: https://github.com/Expensify/Expensify/issues/164496

Upwork job link: https://www.upwork.com/jobs/~01b1a9d276904a90c7

View all open jobs on Upwork

MelvinBot commented 3 years ago

Triggered auto assignment to @trjExpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

MelvinBot commented 3 years ago

Triggered auto assignment to @Jag96 (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

pranshuchittora commented 3 years ago

Proposal 📄

Make button styles consistent in Expensify.cash

Investigation 🕵🏻‍♂️

Issue is due to improper styling

Approach 👨🏼‍💻

File of concern : Profile, Home...

One of the easiest ways is to remove margin and replace it with padding
Refactor the display properties so that items attached to the bottom when the keyboard is opened.

Discussion with the team might be required to pick the best possible approach

Best Practices 💃🏼

No inline styling
Flexibility (changes shouldn't break web)
Follow React Native best practices

Testing Strategy 🧪

Basic render tests

Expected Delivery Time 📦

Approx 1-3 days.

Previous Experience 🙅🏼‍♂️

https://github.com/Expensify/Expensify.cash/issues/2811

Jag96 commented 3 years ago

@pranshuchittora it looks like you have a few other issues currently assigned to you, so going to give others a chance to write up proposals for this.

For the proposals here, please ensure specific details about what style changes are necessary are included.

parasharrajat commented 3 years ago

Here is how I would like to tackle this.

Proposal

To make the button sticky we can use a common technique where we keep the parent flex:1and then add the main content inside a ScrollView and add the fixed button as next sibling element. This is already implemented in the few of the components mentioned above.

For IOS I will wrap all the components in the KeyboardAvoidingView to make the button fixed to bottom.

Skeleton would be something like:

        <ScreenWrapper>
            <KeyboardAvoidingView>
            <HeaderWithCloseButton
            />
            <ScrollView style={styles.flex1}>
            </ScrollView>
            <View style={[styles.ph5, styles.pb5]}>
                <Button
                    success
                    isDisabled={isButtonDisabled}
                    style={[styles.w100]}
                />
            </View>
            </KeyboardAvoidingView>
        </ScreenWrapper>

We use our custom Button component for manage the disable/enable state & look.
We will use [styles.ph5, styles.pb5] to maintain the margin around the button.
Tie up the Button's isDisabled prop with form state so that unless form is valid we keep the button deactivated.
Apply the above steps to all the listed component.

Jag96 commented 3 years ago

@parasharrajat your proposal looks good, just one question: it looks like the skeleton you're proposing is currently implemented on the ProfilePage, but it is one of the buttons on the list that needs to be changed, what changes will you make there?

When testing on the latest version (1.0.55.0) on the iOS Simulator, it doesn't look like the Save button on the Profile Page is shown on top of the keyboard (see image below). @shawnborton can you confirm that we want the Save button to show on top of the keyboard in this case?

shawnborton commented 3 years ago

Confirming, I think the Save button should be visible above the keyboard.

parasharrajat commented 3 years ago

Oh, last time I checked it was above the Keyboard so let me just have a look on the latest code.

parasharrajat commented 3 years ago

@Jag96 Oh sorry I though we are already using the https://github.com/Expensify/Expensify.cash/blob/c11594b845351a638f4782e7e2d6dc0a6772294d/src/libs/KeyboardAvoidingView/index.ios.js. we don't need it for Android.

But it seems we have only added it one place so far so I will wrap all the components in the KeyboardAvoidingView to make the button fixed to bottom. Updating the proposal...

Jag96 commented 3 years ago

@parasharrajat your proposal looks good 👍 Please submit a proposal on Upwork, once it's approved you can start on a PR

michaelhaxhiu commented 3 years ago

Hired @parasharrajat on Upwork. Feel free to start on your PR!

michaelhaxhiu commented 3 years ago

Will pay on June 18 (7 days after PR merged to allow for regression handling)

michaelhaxhiu commented 3 years ago

Payment will be sent tomorrow.

arielgreen commented 3 years ago

@michaelhaxhiu please pay this out ASAP.

arielgreen commented 3 years ago

Ah, nvm, it's paid. Please be sure you're ending contracts. I will do so.

michaelhaxhiu commented 3 years ago

Please be sure you're ending contracts.

Shoot, sorry! I did forget to end the contract.

Expensify / App