[NO QA] Update and rename Add-comments-and-attachments-to-a-report.md to Edit…#57878
[NO QA] Update and rename Add-comments-and-attachments-to-a-report.md to Edit…#57878neil-marcellini merged 7 commits intomainfrom
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? |
|
@neil-marcellini @strepanier03 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://3e8e6a4c.helpdot.pages.dev ⚡️ |
neil-marcellini
left a comment
There was a problem hiding this comment.
Overall looks good. A few small things stood out to me so I want to make sure they're intentional.
docs/articles/expensify-classic/reports/Edit-and-Submit-Expense-Reports.md
Show resolved
Hide resolved
| - **To**: Approver’s name(s). | ||
| - **CC**: Additional recipients (comma-separated). | ||
| - **Memo**: Additional notes. | ||
| - **Attach PDF**: Attach a report copy (optional). |
There was a problem hiding this comment.
"Attach a report copy" I find that wording a little confusing. What about something like "Attach a PDF for additional context"?
There was a problem hiding this comment.
this article had a readability score of 9 and an AI parsing score of 9.5 so i think its fine to leave as-is.
There was a problem hiding this comment.
What kind of PDF would you have that is a copy of the report? I don't think we should allow the rating of an AI to overrule a human opinion outright.
There was a problem hiding this comment.
I'll make this not a blocker, but I would like to see it reworded.
docs/articles/expensify-classic/reports/Edit-and-Submit-Expense-Reports.md
Show resolved
Hide resolved
|
replied and resolved comments! 👍 |
|
ill take a look at the redirect file to get that cleaned up and ping ya once this is ready to merge. |
|
@neil-marcellini - i think ive resolved the redirects issue. not sure if anything else is required for resolving the comments you left, but this should be good to merge as long as the redirects file is good to go. and then for the next couple of weeks, @stephanieelliott and i are optimizing the rest of the site for AgentZero parsing, so no need to scrutinize the updates too much ! communicating so that you don't do extra work that is not required of you! ty ty. |
What was the redirects issue? Either way good to hear and your updates there look fine.
It's my job to review all the changes in PRs. It doesn't take me too much time. I think we still want the help site to work well and make sense throughout this process, so might as well give it a human read through. |
|
No reviewer checklist is required for this help site change. @maddylewis feel free to address the non blocking comment and merge, address it in a follow up, or ignore for now, merge and we can fix it someday if a user is ever confused. |
|
@neil-marcellini - i believe a PR reviewer checklist is required for these! want to add that when you have a chance? tysm!
|
|
Yes the check fails but I'm adding a note explaining why it's irrelevant. It's easier and makes more sense to me than pasting a completed checklist that I didn't actually follow. |
|
🚀 Deployed to staging by https://github.com/neil-marcellini in version: 9.1.11-1 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.1.12-7 🚀
|

…-and-Submit-Expense-Reports.md
https://github.com/Expensify/Expensify/issues/473907
Explanation of Change
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videosundefined