Standardize Button Usages

[marcaaron]

Details

cc @shawnborton This is a refactor of most of our "button" usages so they will use a common Button component to replace the ButtonWithLoader that is pretty underutilized and leads to a lot of duplicate JSX and some style inconsistencies e.g. only certain buttons react to hover styles on web.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/164118

Tests

QA Steps

1. No specific QA, just test various button usages for regressions or any weirdness.

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Screenshots

Including screenshots to verify the button still works, the only change is that buttons will respond to hover on web and very slight change to the animation - since we are no longer using a TouchableOpacity for many buttons.

Web

Mobile Web

Desktop

iOS

Android

[shawnborton]

This is great. Curious what you would do for this kind of button where it has an icon, and the icon is in front of the text string:

[marcaaron]

Curious what you would do for this kind of button where it has an icon, and the icon is in front of the text string

The approach I went with for now would be to pass in the "button content" as a prop.

https://github.com/Expensify/Expensify.cash/blob/b079af398ee5551388d165d6a736686fbd955346/src/pages/settings/Profile/ProfilePage.js#L274-L287

That would effectively make it so a button's content could be "anything".

However, if that's too much flexibility we could embed the icon into a button as an option and then enable it with a boolean prop flag like shouldShowDownCaret. I sorta feel like the decision to add props like that should depend on how many usages of this button style we have.

[shawnborton]

I sorta feel like the decision to add props like that should depend on how many usages of this button style we have.

Totally agree with that, all of this sounds good to me.

[joelbettner]

I don't seee TouchableOpacity used anywhere in the new Button.js component (I could be missing it). Is there a TouchableOpacity.js file that could be deleted just like this ButtonWithLoader.js file?

[marcaaron]

Ah, nope because this component is built into react-native.

[joelbettner]

Looks like a merge conflict cropped up in one of the files. :(

Other than that, it looks good to me.

[marcaaron]

I'll look into why these tests are failing in a sec. Probably someone added a ButtonWithLoader

[marcaaron]

It was me, I added it lol

[deetergp]

Looks pretty straight forward to me.

[marcaaron]

Gonna merge this to prevent conflicts.