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

New breakpoint for tablet size #4793

Merged
merged 2 commits into from
Aug 24, 2021
Merged

New breakpoint for tablet size #4793

merged 2 commits into from
Aug 24, 2021

Conversation

kakajann
Copy link
Contributor

Details

  • Breakpoint added for tablet sizes (800px - 1024px)
  • WorkspaceCardPage is responsive now

Fixed Issues

Fixes #4659

Tests

QA Steps

Tested On

  • Web
  • Mobile Web
  • Tablet

Tablet 801px

Screen Shot 2021-08-24 at 2 31 15 AM

Tablet 1024px

Screen Shot 2021-08-24 at 2 31 23 AM

@kakajann kakajann requested a review from a team as a code owner August 23, 2021 21:36
@MelvinBot MelvinBot requested review from timszot and removed request for a team August 23, 2021 21:36
@@ -1880,6 +1880,12 @@ const styles = {
width: '150%',
},

fullscreenCardTablet: {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like the "tablet" styles are triggered with isMediumScreenWidth. I would suggest we align the words to be consistent here and just use something like fullscreenCardMediumScreen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed all

  • workspaceCardTablet ➡️ workspaceCardMediumScreen
  • fullscreenCardTablet ➡️ fullscreenCardMediumScreen
  • workspaceCardContentTablet ➡️ workspaceCardContentMediumScreen

Copy link
Contributor

@timszot timszot left a comment

Choose a reason for hiding this comment

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

LGTM

@timszot timszot merged commit eb73fd3 into Expensify:main Aug 24, 2021
@OSBotify
Copy link
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.

@isagoico
Copy link

Hello @kakajann, Any QA tests needed for this PR?

@isagoico
Copy link

Tested this by navigating to Workspace > Expensify Card and verifying the different breakpoints and it was a pass.
image
image

@shawnborton @timszot can you confirm these were the QA tests needed for this PR?

@kakajann
Copy link
Contributor Author

Sorry @isagoico, I'll add QA test steps in my next PR's

@shawnborton
Copy link
Contributor

Looks correct to me 👍

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.0.88-2 🚀

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

@mountiny
Copy link
Contributor

@kakajann Great job with this one!

I just wanted to raise awareness about a change we did in our issues/PR management process which helps us track the payment due date automatically. For that, we need the PR description to follow the PR template for this repository.

Specifically, I am talking about the Fixed issues section. We need the issues to be linked without using the github keywords. Simply use dollar sign and copy/paste in the link of the issue. Like so:

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

Then the issue will be automatically updated on deployment and we will not miss paying you on Upwork 🎉

Thank you for keeping that in mind in your next PRs 😊 and keep them coming 🎉

@botify
Copy link

botify commented Sep 1, 2021

This has been deployed to production and is now subject to a 7-day regression period.
If no regressions arise, payment will be issued on 2021-09-08. 🎊

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.

[Hold for Payment] Manage cards button is not visible when viewport is ~840px
7 participants