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

Set button size Medium for Workspace Invite and Resend Link #7224

Merged
merged 2 commits into from
Jan 14, 2022

Conversation

mananjadhav
Copy link
Collaborator

@mananjadhav mananjadhav commented Jan 14, 2022

Details

  • Set button size Medium for Workspace Invite and Resend Link

Fixed Issues

$ #7037

Tests

  • Verify that no errors appear in the JS console

QA Steps

For Workspace Manage Members

  1. Click on the account avatar
  2. Click on the workspace name to navigate to the workspace settings
  3. Click on Manage members to navigate to the manage members page
  4. Size of the "Invite" and "Remove" buttons should be medium (earlier it was small)

For Resend Link

  1. Go to login page
  2. Enter the existing login email address
  3. Click on "Forgot?" in the password form
  4. "Resend" button on the Resend link page should have medium size (earlier it was default)
  • Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web-resend-link-medium-button

web-wkspace-invite-medium-button

Mobile Web

mweb-wkspace-invite-medium-button

mweb-resend-link-medium-button

Desktop

desktop-wkspace-invite-medium-button

desktop-resend-link-medium-button

iOS

ios-resend-link-medium-button

ios-wkspace-invite-medium-button

Android

android-resend-link-medium-button

android-wkspace-invite-medium-button

@mananjadhav mananjadhav marked this pull request as ready for review January 14, 2022 18:43
@mananjadhav mananjadhav requested a review from a team as a code owner January 14, 2022 18:43
@MelvinBot MelvinBot removed the request for review from a team January 14, 2022 18:43
@parasharrajat
Copy link
Member

cc: @shawnborton for button design check...

Copy link
Member

@parasharrajat parasharrajat left a comment

Choose a reason for hiding this comment

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

LGTM. Leaving button design check to Shawn.
cc: @roryabraham

🎀 👀 🎀 C+ reviewed

@shawnborton
Copy link
Contributor

Looks good!

@@ -139,6 +139,7 @@ class ResendValidationForm extends React.Component {
</Text>
</TouchableOpacity>
<Button
medium
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious - if you don't specify medium, what size is the default? I wonder if we want to make medium the default size if it's not already?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It takes styles.button which has the height of large size with ph3 horizontal spacing.

Copy link
Contributor

Choose a reason for hiding this comment

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

This ambiguity would also be removed if we had a size prop with a default value 😏
Not going to address here but in #7220 instead

@@ -139,6 +139,7 @@ class ResendValidationForm extends React.Component {
</Text>
</TouchableOpacity>
<Button
medium
Copy link
Contributor

Choose a reason for hiding this comment

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

This ambiguity would also be removed if we had a size prop with a default value 😏
Not going to address here but in #7220 instead

@roryabraham roryabraham merged commit 0f49d20 into Expensify:main Jan 14, 2022
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @roryabraham in version: 1.1.29-7 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.30-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants