-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix the new message button doesn't work on some comment linkings #46627
Changes from 9 commits
00a7bed
69ce490
4987917
76f9489
7036096
4b4b369
3235453
ceae85f
a9d1374
02fca25
da00fa5
19844ab
7553ed6
abae889
c498c92
0993bd4
c3bc9d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import {PortalHost} from '@gorhom/portal'; | ||
import {useIsFocused} from '@react-navigation/native'; | ||
import type {RouteProp} from '@react-navigation/native'; | ||
import {useIsFocused, useRoute} from '@react-navigation/native'; | ||
import type {StackScreenProps} from '@react-navigation/stack'; | ||
import lodashIsEqual from 'lodash/isEqual'; | ||
import React, {memo, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState} from 'react'; | ||
|
@@ -32,14 +33,14 @@ import useViewportOffsetTop from '@hooks/useViewportOffsetTop'; | |
import Timing from '@libs/actions/Timing'; | ||
import Log from '@libs/Log'; | ||
import Navigation from '@libs/Navigation/Navigation'; | ||
import type {AuthScreensParamList} from '@libs/Navigation/types'; | ||
import clearReportNotifications from '@libs/Notification/clearReportNotifications'; | ||
import Performance from '@libs/Performance'; | ||
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; | ||
import * as ReportActionsUtils from '@libs/ReportActionsUtils'; | ||
import * as ReportUtils from '@libs/ReportUtils'; | ||
import shouldFetchReport from '@libs/shouldFetchReport'; | ||
import * as ValidationUtils from '@libs/ValidationUtils'; | ||
import type {AuthScreensParamList} from '@navigation/types'; | ||
import * as ComposerActions from '@userActions/Composer'; | ||
import * as Report from '@userActions/Report'; | ||
import CONST from '@src/CONST'; | ||
|
@@ -95,7 +96,8 @@ function getParentReportAction(parentReportActions: OnyxEntry<OnyxTypes.ReportAc | |
return parentReportActions[parentReportActionID ?? '0']; | ||
} | ||
|
||
function ReportScreen({route, currentReportID = '', navigation}: ReportScreenProps) { | ||
function ReportScreen({currentReportID = '', navigation}: ReportScreenProps) { | ||
const route = useRoute<RouteProp<AuthScreensParamList, typeof SCREENS.REPORT>>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this change necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @s77rt When a user clicks the same linked message twice (when the current URL path and the clicked link are the same), the using route property of report screen:usginRouteProperty.mp4using useRoute:usingUseRoute.mp4There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this is already working on Screen.Recording.2024-08-04.at.5.14.09.PM.movThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @s77rt Because I am fixing this bug, I am using the same source of route in In your video, the bug does not appear in the main branch because You can reproduce this bug by reverting these two lines in my changes, meaning by using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tsa321 Can you please clarify the solution independently for those two bugs:
(Also please merge |
||
const styles = useThemeStyles(); | ||
const {translate} = useLocalize(); | ||
const reportIDFromRoute = getReportID(route); | ||
|
@@ -782,6 +784,7 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro | |
<ReportActionsView | ||
reportActions={reportActions} | ||
report={report} | ||
route={route} | ||
parentReportAction={parentReportAction} | ||
isLoadingInitialReportActions={reportMetadata?.isLoadingInitialReportActions} | ||
isLoadingNewerReportActions={reportMetadata?.isLoadingNewerReportActions} | ||
|
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.
Why do we fallback to
sortedPages[i - 1]
?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.
Actually I am trying to fix a type-checking error.
result?.at(-1)
gives a type-checking error. Should I useresult[result?.length - 1]
instead?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.
prevPage
is expected to beundefined
at first, no need for the fallbackThere 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.
Alright, since
result?.at(-1)
fails the type check, I will useresult[result?.length - 1]
instead.