-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[feature]: Using the product as a copilot #48594
Conversation
I will open the PR up for review today |
@rushatgabhane 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] |
Backend is deployed, can we do another quick round of tests to make sure everything's matching? Just spot check the tasks, changelogs, that weren't there before. Then I think we should be good to merge and @allgandalf sounds like your 1 day w/out computer is today, which I think will work well if we merge later today, will be on staging tomorrow probably. As for invoices, I think we can handle that in a follow up since we need the details and it came late. If you're okay with handling once we have those details? |
For sure, @DylanDylann can you also help me with this, I will respond to other comments in sometime, just got back |
Oops, sorry my bad 😓 |
I Tested and Verified the following Work:
|
Sure thing, lets follow up when we have more info |
TBH i am really clueless how these system messages really work, I spend half day last week finding code for the messages and found out they come from |
Can you share reproducible steps? I am having a difficulty reproducing that |
Soo, With that I feel, lets merge this one and hope the QA doesn't find any major flaws 🤞 |
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.
let's do it!
@allgandalf can you take a look at this typeerror:
|
actually I think I have it, i can put up a PR in a sec |
#49821 if @DylanDylann or @allgandalf you want to take a look |
🚀 Deployed to staging by https://github.com/dangrous in version: 9.0.41-0 🚀
|
This comment has been minimized.
This comment has been minimized.
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.41-10 🚀
|
onSelected: () => selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.PAY, report?.reportID ?? '-1'), false), | ||
onSelected: () => { | ||
if (isDelegateAccessRestricted) { | ||
setIsNoDelegateAccessMenuVisible(true); |
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.
FYI: we should use Modal.close()
wrapper for this action. Otherwise, the modal may remain stuck.
Context: #56139
Details
Add delegate avatar and delegate message
Fixed Issues
$ #46926
PROPOSAL: https://expensify.slack.com/archives/C02NK2DQWUX/p1725046112539189?thread_ts=1725040927.420699&cid=C02NK2DQWUX
Tests
on behalf of
:Offline tests
on behalf of
:QA Steps
on behalf of
: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.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop