Page MenuHomePhabricator

[web] Create a new modal for the media gallery on web
ClosedPublic

Authored by rohan on Feb 23 2023, 10:09 AM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 21 2024, 2:05 PM
Unknown Object (File)
Feb 21 2024, 10:22 AM
Unknown Object (File)
Feb 21 2024, 9:06 AM
Unknown Object (File)
Feb 21 2024, 9:06 AM
Unknown Object (File)
Feb 21 2024, 9:06 AM
Unknown Object (File)
Feb 21 2024, 9:06 AM
Unknown Object (File)
Feb 19 2024, 10:54 PM
Unknown Object (File)
Feb 19 2024, 10:53 PM
Subscribers

Details

Summary

The web client needs a modal to contain all of the shared media for a specific thread. Much of the logic here is the same that is in the native counterpart, D6659.

Linear: https://linear.app/comm/issue/ENG-2878/create-a-new-modal-for-the-media-gallery-on-web

Depends on D6720

Test Plan

The next diff will add the media modal to the thread menu to click through into, but I will show a video of what the modal looks like on web below.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Just some nits

web/modals/threads/gallery/thread-settings-media-gallery.react.js
59–73 ↗(On Diff #23020)

All of these return keywords (along with the curly braces) can be removed. When you have a function that just returns immediately, it's cleaner to skip the return

80–83 ↗(On Diff #23020)

I would reverse this condition and "early exit" so you can decrease indentation. More reading

Reverse the handleScroll if condition to early return and format code

ashoat added inline comments.
web/modals/threads/gallery/thread-settings-media-gallery.react.js
56 ↗(On Diff #23037)

I would either replace this line with an invariant, or I would remove the condition on line 49 and let it default to this line

This revision is now accepted and ready to land.Feb 24 2023, 6:12 AM

Address feedback on filteredMediaInfos