-
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
Update Pay an Invoice and Send an Invoice FAQ section so it collapses #49135
Conversation
trying to remove the duplicate FAQ section from the side bar
Updating the FAQ section so it collapses
@mjasikowski 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] |
A preview of your ExpensifyHelp changes have been deployed to https://head.helpdot.pages.dev ⚡️ |
Creating a new webpage for workflows
@Christinadobrzyn can you confirm that your changes worked as expected in the test deploy? #49135 (comment) |
Reviewed without checklist, my bad
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.
Just one small change, but it would be also great if you could directly link to the slack or GitHub discussion relevant to the issue in the description, so we can see the context.
@@ -62,7 +62,7 @@ Only workspace admins can send invoices. Invoices can be sent directly from Expe | |||
If you have not [connected a business bank account](https://help.expensify.com/articles/new-expensify/expenses-&-payments/Connect-a-Business-Bank-Account) to receive invoice payments, you will see an **Invoice balance** in your [Wallet](https://help.expensify.com/articles/new-expensify/expenses-&-payments/Set-up-your-wallet). Expensify will automatically transfer these invoice payments once a business bank account is connected. | |||
|
|||
|
|||
# FAQs | |||
{% include faq-begin.md %} |
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.
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.
Thanks @mjasikowski - do I need to approve the change or submit a new PR, I'm not finding that DIV anywhere in the changes
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.
@Christinadobrzyn can you edit this PR to have {% include faq-end.md %}
on line 93 of docs/articles/new-expensify/expenses-&-payments/Send-an-invoice.md
? That's what @mjasikowski is saying that we are missing 👍
You added {% include faq-begin.md %}
on line 65 but if you add that you also need to include a corresponding {% include faq-end.md %}
in order to end the FAQ and have it styled correctly.
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.
Ah yes, done! I added {% include faq-end.md %} to the end of both FAQ edits.
Gonna try to resolve that checklist error - I added it again in the OP but that didn't resolve it. Going to add it here as a new comment to see if that works. PR Author Checklist
|
@Christinadobrzyn the failing check is because neither @mjasikowski or I have done the checklist as a reviewer, you're good on the Author side |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
removed <div> from the end of the article (line 56 or something) and added {% include faq-end.md %}
removed <div> and added {% include faq-end.md %} to the end of the article
Added {% include faq-end.md %} to the end of the FAQ section
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.
Couple more tweaks, getting close.
</div> | ||
{% include faq-end.md %} |
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.
Can you revert this? We already have a {% include faq-end.md %}
on line 56 here, and we also need the </div>
tag in order to close the opening <div>
tag from line 5 of this file.
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.
By "revert" I mean add the div tag back and get rid of the {% include faq-end.md %}
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.
Ah okay, I updated the article! TY!
|
||
</div> | ||
{% include faq-end.md %} |
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.
We don't want to delete the </div>
. We have an opening <div>
tag on line 5 of this file, and this is its closing tag. Please see this article for an example of what we want.
It should be like this:
{% include faq-end.md %}
</div>
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.
Ah, okay, I updated the article!
added this to the end of the article. {% include faq-end.md %} </div>
added this to the end of the article {% include faq-end.md %} </div>
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.
LGTM! All yours @mjasikowski
Related to GH - Expensify/Expensify#427929 Adding images to the article - https://help.expensify.com/articles/new-expensify/expenses-&-payments/Pay-an-invoice
Hi! just a side note that I added two images to this PR as part of this GH https://github.com/Expensify/Expensify/issues/427929 so I don't know if you need to review those additional parts of this PR @blimpich |
@mjasikowski all yours |
all good! |
🚀 Deployed to staging by https://github.com/mjasikowski in version: 9.0.40-0 🚀
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.0.40-6 🚀
|
Details
Updating the FAQ section to remove this dupe mention
Also making the Send an Invoice FAQ section collapsable
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.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 and/or tagged@Expensify/design
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.