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

Update the height of Edit profile button #3854

Merged
merged 2 commits into from
Jul 2, 2021
Merged

Conversation

mountiny
Copy link
Contributor

@mountiny mountiny commented Jul 1, 2021

Details

This PR is adding a small option to Button component (styles already exist) and updates the Edit profile button in Profile part of Settings to be 28px tall instead of 40px.

Fixed Issues

$ #2434

Tests

  1. Run the app
  2. Click on the profile icon
  3. Click on profile
  4. Make sure the Edit profile button is 28px tall

QA Steps

Same as testing except of visiting staging or production version of the app.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

simulator_screenshot_C41CFD88-3CAE-4386-B082-8E4A9F9B47C3

Desktop

image

iOS

simulator_screenshot_AB719E40-ACEA-42D3-83DE-0C8A5D72C248

Android

@mountiny mountiny requested a review from a team as a code owner July 1, 2021 15:58
@MelvinBot MelvinBot requested review from HorusGoul and removed request for a team July 1, 2021 15:59
@mountiny
Copy link
Contributor Author

mountiny commented Jul 1, 2021

@shawnborton Hi! Would you be able to double check the design here. From the issue, I was not sure if we should update the height of the button on all platforms or only website/desktop. Thank you for feedback!

@shawnborton
Copy link
Contributor

Out of curiosity, what font size do small buttons use?

@shawnborton
Copy link
Contributor

I agree this should be a global change and not platform specific (as are most of the things we do with E.cash!)

@mountiny
Copy link
Contributor Author

mountiny commented Jul 1, 2021

Ok, so on the screenshots, the text is size 15px, because the styles were no applied on correct component. So for comparison, this is font-size: 15px as it was before with the height of button set to 40px:

image

and this is the actual buttonSmallText font-size: 11px as it was already defined in the codebase:

image

Let me know which one I should use :)

@shawnborton
Copy link
Contributor

Perfect! Yes - the smaller button size should use the smaller font size as well.

@mountiny
Copy link
Contributor Author

mountiny commented Jul 1, 2021

Thank you Shawn! Updated it and also the screenshots in the description.

@HorusGoul It is good for a review 🙏

@HorusGoul HorusGoul merged commit 5653309 into main Jul 2, 2021
@HorusGoul HorusGoul deleted the vit-updateEditPhotoButton branch July 2, 2021 11:09
@OSBotify
Copy link
Contributor

OSBotify commented Jul 2, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@roryabraham
Copy link
Contributor

Unfortunately the deploy comments are not working right now. This was deployed to staging yesterday.

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