-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Remove unused report functions #12357
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
Conversation
| super(props); | ||
|
|
||
| Timing.start(CONST.TIMING.HOMEPAGE_INITIAL_RENDER); | ||
| Timing.start(CONST.TIMING.HOMEPAGE_REPORTS_LOADED); |
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.
This timer was being started but never stopped / recorded, because the stop for it was in fetchAllReports, which was no longer being called anywhere.
| // and the report does not yet exist in Onyx (eg. a new DM created with the logged in person) | ||
| if (report.reportID && report.reportName === undefined) { | ||
| fetchChatReportsByIDs([report.reportID]); | ||
| openReport(report.reportID); |
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.
This is the only risky change – need to make extra sure that openReport will do everything that fetchChatReportsByIDs once did
|
Closing in favor of #12157 |
Details
This PR removes some unused and/or outdated code I stumbled upon.
Fixed Issues
$ n/a
Tests
TBD / in progress
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)Screenshots
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android