-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Update Configure-Netsuite.md #54390
Update Configure-Netsuite.md #54390
Conversation
Concierge reviewer checklist:
For more detailed instructions on completing this checklist, see How do I review a HelpDot PR as a Concierge Team member? |
@puneetlath @Christinadobrzyn One of you needs to 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] |
A preview of your ExpensifyHelp changes have been deployed to https://da9294da.helpdot.pages.dev ⚡️ |
@Christinadobrzyn -- preview link - https://139b6e29.helpdot.pages.dev/articles/expensify-classic/connections/netsuite/Configure-Netsuite the update here is getting images added to the article. those were added successfully 👍 |
@maddylewis I have the PR to review. The article is amazing! I just double-checked the screenshots against my test account, and it looks like the Export tab might need to be updated to show the accounting method. Also I think the Advanced tab screenshot needs to be updated. It looks like we removed Export Foreign Currency Amount. Here's what I see in my test account, if it's helpful. So I think we also need to remove the Export Foreign Currency Amount section from the body of the help article and add Export To Next Open Period. Let me know if you need any help with that! |
@Christinadobrzyn is ok, thank you for that in-depth review!! i will need to request new images from the design team. which i likely won't do until after the holidays. once i have the updated image file, i will add it to the site by creating a separate PR. then, once the new image files are on the repo, i will update this file with the new images. |
Awesome! That totally sounds good @maddylewis. I agree that getting this help article live is important since there are changes that customers might have questions about. Do you want to remove the Export Foreign Currency Amount section from the help article now and we'll add the section about Export To Next Open Period and add the new screenshots after the holidays? |
@Christinadobrzyn - sure! fwiw the resource is currently live and the only change was adding those images. but yes, let me remove the Export Foreign Currency section real quick. |
removed out of date images for now and removed mention of Export Foreign Currency
Concierge reviewer checklist:
For more detailed instructions on completing this checklist, see How do I review a HelpDot PR as a Concierge Team member? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Looks great! The decision is to update the screenshots on another PR after running them past Design. We're going to keep the Export Foreign Currency section since the design doc indicates that might be added back in future iterations. Adding Pullerbear based on the SO, although that might also be you, @puneetlath! |
added info about foreign currency back + added advanced tab image back since that feature will likely be available again soon.
@maddylewis if you add the GH issue link next to the dollar sign here, then you'll get comments on the issue when the PR is deployed and the issue will be automatically closed. |
@puneetlath - ahhh thank you. i think i will write out all of these various PR tips in SO. they're so helpful but ya just don't know until you know! (as a non-eng) ! |
No problem! |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.0.78-0 🚀
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.0.78-0 🚀
|
2 similar comments
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.0.78-0 🚀
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.0.78-0 🚀
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.0.78-0 🚀
|
1 similar comment
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.0.78-0 🚀
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.0.78-0 🚀
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.78-6 🚀
|
Explanation of Change
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/433121
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
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)/** comment above it */
this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)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/Videosundefined