-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Add the Connections
section on the More Features
page
#38900
Conversation
@cubuspl42 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-03-29.at.2.50.32.AM.movAndroid: mWeb ChromeScreen.Recording.2024-03-29.at.2.39.28.AM.moviOS: NativeScreen.Recording.2024-03-29.at.2.35.16.AM.moviOS: mWeb SafariScreen.Recording.2024-03-29.at.2.32.55.AM.movMacOS: Chrome / SafariScreen.Recording.2024-03-29.at.2.09.12.AM.movMacOS: DesktopScreen.Recording.2024-03-29.at.2.28.46.AM.mov |
@cubuspl42, I assigned @allroundexperts who were available when I opened the PR as this required a quick review 🙇 |
BUG Offline mode does not work. Screen.Recording.2024-03-29.at.2.09.51.AM.mov |
@allroundexperts, I checked but I didn't see any console log errors about failed API calls when I toggled the accounting feature. I do see errors when enable any other features while the device is offline though 🤔 Screen.Recording.2024-03-28.at.2.16.06.PM.mov |
@hayata-suenaga The bug here is that when you're offline and you toggle this new option, its not greyed out. Other options are greyed out if they're toggled when offline. |
ah I understand the issue now. thank you for the clarification 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works good!
@tylerkaraszewski Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me :)
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/srikarparsi in version: 1.4.58-0 🚀
|
🚀 Deployed to production by https://github.com/Beamanator in version: 1.4.58-8 🚀
|
Why did we call this |
oops fixing now thanks for the check 🙇 we use the word |
ah now I see what happened! I used an already-existing display text that was added in another PR for the accounting connections feature. and I forgot to check if they were correct or not. |
Details
This PR adds a new section with a new item for toggling the Accounting Connection feature.
Fixed Issues
$ #38899
PROPOSAL: N/A Internal PR
Tests / Offline tests / QA Steps
accountingOnNewExpensify
betaPR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectionThis PR just adds a new item using already existing components. I only included a screenshot for the web.
toggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop