[NoQA] refactor: Split ReportActionCompose into more compositional components to allow for separate EditOnlyReportActionCompose#90915
Conversation
|
@hungvu193 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] |
ReportActionCompose into more compositional components to allow for separate EditOnlyReportActionComposeReportActionCompose into more compositional components to allow for separate EditOnlyReportActionCompose
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
ReportActionCompose into more compositional components to allow for separate EditOnlyReportActionComposeReportActionCompose into more compositional components to allow for separate EditOnlyReportActionCompose
|
Let's handle this one next deploy 😄 |
|
I'm a bit under the water 😭 |
ReportActionCompose into more compositional components to allow for separate EditOnlyReportActionComposeReportActionCompose into more compositional components to allow for separate EditOnlyReportActionCompose
…-report-action-compose
|
No product review needed |
|
@hungvu193 How you getting on? |
|
I'll take a look today 🫡 |
1 similar comment
|
I'll take a look today 🫡 |
There was a problem hiding this comment.
I'm definitely not against this idea. The only concern is that, we're passing reportID from component to component, it's kind of hard to chase its absolute parent sometimes. Since they are all children of ComposerProvider, do you think we should let the children use the reportID of ComposerProvider as a single source of truth?
There was a problem hiding this comment.
I also thought about this before, but maybe @adhorodyski can explain why he designed the new ReportActionCompose compositional + context structure like that. We should be able to just pass the reportID to ReportActionCompose once and then re-use it in all sub-components, right?
There was a problem hiding this comment.
Yeah, but since it's a stable property it's not a big deal to either pass it (kinda ugly but I did it myself :D) or share through context. I'm also not against any of these options since there's no harm in either.
There was a problem hiding this comment.
so totally ok to only pass it to the context provider and read from there 👍🏼
…-report-action-compose
|
@hungvu193 ready for review! |
|
Thanks! I'm on it |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
| import ComposerActionMenu from './ComposerActionMenu'; | ||
| import {useComposerEditState} from './ComposerContext'; | ||
| import ComposerEditingButtons from './ComposerEditingButtons'; | ||
|
|
||
| function ComposerActionButton() { | ||
| const {isEditingInComposer} = useComposerEditState(); | ||
|
|
||
| if (isEditingInComposer) { | ||
| return <ComposerEditingButtons />; | ||
| } | ||
| return <ComposerActionMenu />; | ||
| } | ||
|
|
||
| export default ComposerActionButton; |
There was a problem hiding this comment.
App crashes immediately on start.
import React from 'react'; is missing on this file.
|
This is crashing main too. I'll put up a PR to fix it |
|
PR up |
|
Thanks! |
|
@hungvu193 I see you did not include any recordings in your checklist now, please make sure to always test the changes even if its decomposition refactoring in case things like this are missed |
|
I imagine this probably happened during some conflict resolution later so that is unfortunate, but still |
|
@mountiny NO QA steps, OK to check off this one? |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.83-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This is a pure internal code refactor that splits @chrispader, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
@luacmartins @mountiny Thanks for handling this while i was gone! 🙌🏼 |
|
Yeah noQA |
ReportActionCompose into more compositional components to allow for separate EditOnlyReportActionComposeReportActionCompose into more compositional components to allow for separate EditOnlyReportActionCompose
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.83-3 🚀
|

@mountiny @hungvu193
Explanation of Change
Refactors the
ReportActionComposeinto more compositionalsub-components, to prevent the need for thisisEditOnlyboolean flag, which doesn't comply with React compositional best practices.Fixed Issues
$ #90889
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)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
MacOS: Chrome / Safari
Screen.Recording.2026-05-17.at.16.24.14.mov