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

Refactor workspace settings #5642

Merged
merged 135 commits into from
Oct 8, 2021
Merged

Refactor workspace settings #5642

merged 135 commits into from
Oct 8, 2021

Conversation

tgolen
Copy link
Contributor

@tgolen tgolen commented Oct 4, 2021

This PR adds a lot of feature promotion to the workspace settings

Fixed Issues

$ #5689

Tests

  1. Log into NewDot
  2. Create a workspace
  3. Go to Settings > Workspace
  4. Navigate around all the pages and see that most features are locked without a bank account
  5. Add a bank account
  6. Navigate around all the pages and see that the features are now unlocked

QA Steps

Tests are all written in this doc and will be performed internally

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

image

Desktop

image

iOS

image

image

Android

marcaaron
marcaaron previously approved these changes Oct 8, 2021
@marcaaron marcaaron merged commit 2ae2fd8 into main Oct 8, 2021
@marcaaron marcaaron deleted the tgolen-workspace-settings branch October 8, 2021 19:22
github-actions bot pushed a commit that referenced this pull request Oct 8, 2021
Refactor workspace settings

(cherry picked from commit 2ae2fd8)
AndrewGable added a commit that referenced this pull request Oct 9, 2021
@OSBotify
Copy link
Contributor

OSBotify commented Oct 9, 2021

🚀 Cherry-picked to staging by @marcaaron in version: 1.1.7-2 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@isagoico
Copy link

We found this issue while QAing this PR #5760 Not sure if a deploy blocker tho, added the label so it can be decided there

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @AndrewGable in version: 1.1.7-24 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @marcaaron in version: 1.1.7-25 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.8-9 🚀

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

@@ -1861,6 +1872,10 @@ const styles = {
cursor: 'not-allowed',
},

cursorPointer: {
cursor: 'pointer',
Copy link
Member

Choose a reason for hiding this comment

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

This style property is not supported on RN. It is a browser-only css property. Because we used the same styles for all platforms, it caused a console error on native platforms #20589.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we could fix this generically by passing all styles through StyleSheet.create?

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.