[No QA] [BYOC] [Bulk Card Assignments] Release 1+2: Loading states#78167
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
melvin-bot
bot
requested review from
Gonals and
JmillsExpensify
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| return {cardName, customCardName, isCardDeleted, isAssigned, assignedCard, cardholder, failedCompanyCardAssignment}; | ||
| }) ?? []; | ||
| return {cardName, customCardName, isCardDeleted, isAssigned, assignedCard, cardholder, failedCompanyCardAssignment}; |
There was a problem hiding this comment.
❌ PERF-4 (docs)
The cardFeedIcon JSX element is created on every render without memoization, and is passed as a prop to WorkspaceCompanyCardTableItem inside renderItem. This creates a new object reference on every render, causing unnecessary re-renders of list items.
Wrap the cardFeedIcon creation in useMemo with appropriate dependencies:
const cardFeedIcon = useMemo(
() => (
<CardFeedIcon
key={feedName}
iconProps={{
height: variables.cardIconHeight,
width: variables.cardIconWidth,
additionalStyles: styles.cardIcon,
}}
selectedFeed={feedName}
/>
),
[feedName, styles.cardIcon, variables.cardIconHeight, variables.cardIconWidth],
);
...orkspace/companyCards/WorkspaceCompanyCardsTable/WorkspaceCompanyCardsTableHeaderButtons.tsx
Outdated
Show resolved
Hide resolved
tgolen
changed the title
tgolen
merged commit a1feaa2
into
Expensify:byoc-bulk-card-assign-r1
|
🚀 Deployed to staging by https://github.com/tgolen in version: 9.2.86-0 🚀
|
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.86-4 🚀
|
| let cardFeedType: CardFeedType = 'customFeed'; | ||
| if (isPlaidCardFeed) { | ||
| cardFeedType = 'directFeed'; | ||
| } |
There was a problem hiding this comment.
Not only plaid connected feeds are direct, we also have native direct connections. This caused this regression https://expensify.slack.com/archives/C05LX9D6E07/p1767116486536099
4 participants
@tgolen @carlosmiceli
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 stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari