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

Aligned header #7512

Merged
merged 1 commit into from
Feb 2, 2022
Merged

Aligned header #7512

merged 1 commit into from
Feb 2, 2022

Conversation

mateusbra
Copy link
Contributor

@mateusbra mateusbra commented Feb 1, 2022

Details

Style fix for settings subpages miss-aligned headers on IOS and Android

Fixed Issues

$ #7491

Tests | QA Steps

  1. Login in New Dot
  2. Go to Settings
  3. Check header title is vertically centered on all pages where there is a header. E.g. Settings pages.
  4. Note: that it should not affect the header where the subtitle is also visible. e.g. Attachment modal.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Chrome:
image
Safari:
Captura de Tela 2022-02-01 às 18 39 43

Mobile Web

Android chrome mWeb:

image
IOS safari mWeb:
Captura de Tela 2022-02-01 às 19 11 07

Desktop

Captura de Tela 2022-02-01 às 18 22 38

iOS

Captura de Tela 2022-02-01 às 14 19 58

Android

image

@mateusbra mateusbra requested a review from a team as a code owner February 1, 2022 21:27
@MelvinBot MelvinBot requested review from mountiny and parasharrajat and removed request for a team February 1, 2022 21:27
Copy link
Contributor

@mountiny mountiny left a comment

Choose a reason for hiding this comment

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

@mateusbra Could you please also add a screenshot for Safari iOS in the mobile Web category? Not a big deal but the styles sometimes get screwed for mobile safari.

@mateusbra
Copy link
Contributor Author

@mountiny sure!

@mateusbra
Copy link
Contributor Author

mateusbra commented Feb 1, 2022

@mountiny PR updated with Safari IOS on Web and mWeb categories Screenshots

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.

Please change the test steps:
3. Observe that all the subpages are aligned

  1. Check header title is vertically centered on all pages where there is a header. E.g. Settings pages.
  2. Note: that it should not affect the header where the subtitle is also visible. e.g. Attachment modal.

cc: @mountiny
🎀 👀 🎀 C+ reviewed

@mountiny mountiny merged commit ad87352 into Expensify:main Feb 2, 2022
@OSBotify
Copy link
Contributor

OSBotify commented Feb 2, 2022

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

@mountiny
Copy link
Contributor

mountiny commented Feb 2, 2022

@mateusbra Thank you for working on this! 🙇 and @parasharrajat for a review!

@OSBotify
Copy link
Contributor

OSBotify commented Feb 2, 2022

🚀 Deployed to staging by @mountiny in version: 1.1.35-0 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Feb 4, 2022

🚀 Deployed to production by @sketchydroide in version: 1.1.35-1 🚀

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

@mountiny
Copy link
Contributor

mountiny commented Feb 6, 2022

@mateusbra For future PRs, please, make sure the linked issue is written using the following format:

$ https://github.com/Expensify/App/issues/7491

What you have there now is:

$ [#7491](https://github.com/Expensify/App/issues/7491)

We have automation in place, which requires this format to work properly (for example the issue has not been updated with a date when we should pay you). Thank you for keeping your eye on this in future!

@mateusbra
Copy link
Contributor Author

@mountiny sure! Sorry for that. I'll ensure future PR's will follow the correct format 😅

@mountiny
Copy link
Contributor

mountiny commented Feb 6, 2022

No problem at all! 🙇

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.

4 participants