Page MenuHomePhabricator
Differential D4042

[web] Memoize `NotificationsOption` `onSelect` prop w/ `useCallback` hook
ClosedPublic
Actions

Authored by atul on May 15 2022, 7:44 AM.
Tags
None
Referenced Files
F3174380: D4042.id12757.diff
Thu, Nov 7, 4:25 PM
F3172103: D4042.id12756.diff
Thu, Nov 7, 2:23 PM
Unknown Object (File)
Thu, Oct 17, 7:34 PM
Unknown Object (File)
Thu, Oct 17, 7:34 PM
Unknown Object (File)
Thu, Oct 17, 7:34 PM
Unknown Object (File)
Thu, Oct 17, 7:34 PM
Unknown Object (File)
Thu, Oct 17, 7:34 PM
Unknown Object (File)
Thu, Oct 17, 7:34 PM
View All Files
Subscribers
• abosh
• adrian
ashoat

Details

Reviewers
• jacek
tomek
• benschac
• abosh
Commits
rCOMMfc48c1d48bfe: [web] Memoize `NotificationsOption` `onSelect` prop w/ `useCallback` hook
rCOMM2c86cbca6079: [web] Memoize `NotificationsOption` `onSelect` prop w/ `useCallback` hook
Summary

The NotificationsOption onSelect callback prop was being defined inline instead of being memoized with React.useCallback. This diff just fixes that.

Test Plan

Clicked through the NotificationsModal and things worked as expected.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.May 15 2022, 7:44 AM
Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptMay 15 2022, 7:44 AM
atul published this revision for review.May 15 2022, 7:46 AM
Harbormaster completed remote builds in B9118: Diff 12683.May 15 2022, 7:50 AM
atul added a reviewer: • abosh.May 16 2022, 4:50 AM
• abosh accepted this revision.May 16 2022, 8:12 AM
This revision is now accepted and ready to land.May 16 2022, 8:12 AM
atul added 1 blocking reviewer(s): tomek.May 16 2022, 8:39 AM
This revision now requires review to proceed.May 16 2022, 8:39 AM
tomek accepted this revision.May 16 2022, 9:12 AM
This revision is now accepted and ready to land.May 16 2022, 9:12 AM
atul updated this revision to Diff 12752.May 16 2022, 9:48 AM

rebase

Harbormaster completed remote builds in B9170: Diff 12752.May 16 2022, 9:53 AM
atul added inline comments.May 16 2022, 9:54 AM
web/modals/threads/notifications/notifications-modal.react.js
44–45 ↗(On Diff #12752)

This was prettier

Closed by commit rCOMM2c86cbca6079: [web] Memoize `NotificationsOption` `onSelect` prop w/ `useCallback` hook (authored by atul). · Explain WhyMay 16 2022, 9:54 AM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM2c86cbca6079: [web] Memoize `NotificationsOption` `onSelect` prop w/ `useCallback` hook.
atul added a reverting change: rCOMM89cdc5eb4701: Revert "[web] Memoize `NotificationsOption` `onSelect` prop w/ `useCallback`….May 16 2022, 10:09 AM
atul reopened this revision.May 16 2022, 10:10 AM

Ran into strange prettier issue when landing this. Will update after investigating

This revision is now accepted and ready to land.May 16 2022, 10:10 AM
atul updated this revision to Diff 12756.May 16 2022, 10:55 AM

rebase after fixing prettier/eslint issue

Harbormaster completed remote builds in B9172: Diff 12756.May 16 2022, 11:01 AM
Closed by commit rCOMMfc48c1d48bfe: [web] Memoize `NotificationsOption` `onSelect` prop w/ `useCallback` hook (authored by atul). · Explain WhyMay 16 2022, 11:03 AM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMfc48c1d48bfe: [web] Memoize `NotificationsOption` `onSelect` prop w/ `useCallback` hook.

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
notifications/
25 lines

Diff 12757

View Options

web/modals/threads/notifications/notifications-modal.react.js

Loading...