-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 mark message from notification #41484
Merged
blimpich
merged 17 commits into
Expensify:main
from
wildan-m:wildan/fix-34829-mark-message-from-notification
May 13, 2024
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
6023e76
Fix issue message unread from notification center panel
wildan-m d633d44
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m 2d65227
create specific platform file for navigateFromNotification
wildan-m 4172369
fix prettier
wildan-m bed6027
fix typecheck
wildan-m 1a806f1
make isFromNotification more readable
wildan-m 05cadb2
Add comment about referrer param
wildan-m 97c81cf
restore accidentally removed code
wildan-m f58c570
fix typecheck
wildan-m 484249d
fix prettier and typecheck
wildan-m df6fddb
restore accidentaly removed code
wildan-m e66807f
Update src/pages/home/report/ReportActionsList.tsx
wildan-m a9a5840
Add referrer to report screen type
wildan-m 4057c0d
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m 6b88b0f
Change 'notification' to const
wildan-m b2f651b
run prettier
wildan-m 1ad9964
Merge branch 'main' of https://github.com/wildan-m/App into wildan/fi…
wildan-m File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import Navigation from '@libs/Navigation/Navigation'; | ||
import CONST from '@src/CONST'; | ||
import ROUTES from '@src/ROUTES'; | ||
|
||
const navigateFromNotification = (reportID: string) => { | ||
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(reportID, undefined, CONST.REFERRER.NOTIFICATION)); | ||
}; | ||
|
||
export default navigateFromNotification; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import Navigation from '@libs/Navigation/Navigation'; | ||
import ROUTES from '@src/ROUTES'; | ||
|
||
const navigateFromNotification = (reportID: string) => { | ||
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(reportID)); | ||
}; | ||
|
||
export default navigateFromNotification; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -249,10 +249,14 @@ function ReportActionsList({ | |
if (!userActiveSince.current || report.reportID !== prevReportID) { | ||
return; | ||
} | ||
|
||
if (ReportUtils.isUnread(report)) { | ||
if (Visibility.isVisible() && scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) { | ||
// On desktop, when the notification center is displayed, Visibility.isVisible() will return false. | ||
// Currently, there's no programmatic way to dismiss the notification center panel. | ||
// To handle this, we use the 'referrer' parameter to check if the current navigation is triggered from a notification. | ||
const isFromNotification = route?.params?.referrer === CONST.REFERRER.NOTIFICATION; | ||
if ((Visibility.isVisible() || isFromNotification) && scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) { | ||
Report.readNewestAction(report.reportID); | ||
Navigation.setParams({referrer: undefined}); | ||
Comment on lines
+257
to
+259
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. Coming from #45093 we should only set navigation param referrer to undefined when user is linked through notification |
||
} else { | ||
readActionSkipped.current = true; | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Coming from this issue
In most cases, Visibility.isVisible() returns the value that the app is not visible (inactive). It happens because of the leverage of outdated value.
As result chat is not marked as read (chat is bold) on LHN when opening a new message via notification and navigating to another chat.
So the more preferable value is to use isVisible in this screen