Skip to content

Aligned header#7512

Merged
mountiny merged 1 commit into
Expensify:mainfrom
mateusbra:aligned-header
Feb 2, 2022
Merged

Aligned header#7512
mountiny merged 1 commit into
Expensify:mainfrom
mateusbra:aligned-header

Conversation

@mateusbra

@mateusbra mateusbra commented Feb 1, 2022

Copy link
Copy Markdown
Contributor

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

@mountiny mountiny left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

@mountiny sure!

@mateusbra

mateusbra commented Feb 1, 2022

Copy link
Copy Markdown
Contributor Author

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

@parasharrajat parasharrajat left a comment

Copy link
Copy Markdown
Member

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

OSBotify commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

✋ 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

mountiny commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

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

@OSBotify

OSBotify commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

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

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

@OSBotify

OSBotify commented Feb 4, 2022

Copy link
Copy Markdown
Contributor

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

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

@mountiny

mountiny commented Feb 6, 2022

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor Author

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

@mountiny

mountiny commented Feb 6, 2022

Copy link
Copy Markdown
Contributor

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