Skip to content

Revert "refactor: the new-transaction highlight rail and the wide RHP width model" - #100778

Closed
blimpich wants to merge 1 commit into
mainfrom
revert-98744-refactor-Reports-newTransactionHighlight-and-RHPLayout-followups
Closed

blimpich wants to merge 1 commit into
mainfrom
revert-98744-refactor-Reports-newTransactionHighlight-and-RHPLayout-followups

Conversation

@blimpich

@blimpich blimpich commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@blimpich
blimpich requested review from a team as code owners September 9, 2026 23:33
@melvin-bot
melvin-bot Bot requested review from joekaufmanexpensify and removed request for a team September 9, 2026 23:33
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-09T23:46:20.885085Z 0a4b1bd PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@melvin-bot
melvin-bot Bot requested review from marcaaron and removed request for a team September 9, 2026 23:34
@melvin-bot

melvin-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

@marcaaron 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]

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/CONST/index.ts 91.22% <ø> (ø)
...equestReportView/MoneyRequestReportActionsList.tsx 60.44% <100.00%> (ø)
...portActionItem/MoneyRequestReportPreview/index.tsx 95.80% <100.00%> (ø)
...onents/ScreenWrapper/ScreenWrapperStatusContext.ts 100.00% <ø> (ø)
src/components/ScreenWrapper/index.tsx 92.68% <100.00%> (ø)
...archList/ListItem/TransactionGroupListExpanded.tsx 68.18% <ø> (ø)
...hooks/useResponsiveLayoutOnWideRHP/index.native.ts 100.00% <ø> (ø)
src/hooks/useScreenWrapperTransitionStatus.ts 75.00% <100.00%> (ø)
src/libs/actions/IOU/Split.ts 77.13% <100.00%> (+0.09%) ⬆️
src/libs/actions/IOU/TrackExpense.ts 85.03% <100.00%> (+0.04%) ⬆️
... and 14 more
... and 11 files with indirect coverage changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a4b1bdfbe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +45 to +49
if (transactions !== undefined && prevTransactions !== undefined && transactions.length > prevTransactions.length) {
if (skipFirstTransactionsChange.current) {
skipFirstTransactionsChange.current = false;
} else {
diffTransactions = transactions.filter((transaction) => !prevTransactions?.some((prevTransaction) => prevTransaction.transactionID === transaction.transactionID));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset the diff baseline when switching reports

When the prev/next carousel changes reportID on the mounted screen, prevTransactions still belongs to the previous report. If the destination report has more transactions, this branch classifies every destination transaction absent from the old report as newly added, causing an incorrect scroll and highlight across the report. Reset or re-baseline the comparison whenever reportID changes.

Useful? React with 👍 / 👎.

Comment on lines 72 to 75
const nonRepeatableProgress = useSharedValue(initialNonRepeatableProgressValue);
const {didScreenTransitionEnd, shouldUseNarrowLayoutOnWideRHP} = useScreenWrapperTransitionStatus();
const navigation = useContext(NavigationContext);
const {didScreenTransitionEnd} = useScreenWrapperTransitionStatus();
const theme = useTheme();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Disarm queued highlights when the row stops qualifying

If shouldHighlight becomes false after startHighlight is armed but before the screen transition finishes, this early return leaves startHighlight true. Once didScreenTransitionEnd changes, the second effect still runs the entry and pulse animation without rechecking shouldHighlight, so a cleared or recycled row can flash as newly added. Clear the armed state or verify the current predicate before starting the animation.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant