Page MenuHomePhabricator
Differential D7994

[web] Add MediaGalleryItem component
ClosedPublic
Actions

Authored by bartek on Fri, May 26, 1:17 AM.
Tags
None
Referenced Files
F560005: a3d6c8.png
Fri, May 26, 8:44 AM
F560004: 6b1ca2.png
Fri, May 26, 8:44 AM
Unknown Object (File)
Fri, May 26, 2:38 AM
Subscribers
ashoat
tomek

Details

Reviewers
inka
ginsu
atul
kuba
Commits
rCOMMf599a6ea375d: [web] Add MediaGalleryItem component
Summary

Proper displaying of encrypted and non-encrypted thread gallery media required more code so I extracted it into a separate component.
Basically, most of the work is already done by the <EncryptedMultimedia> component so I only needed to add thumbhash and background for non-encrypted images.

Depends on D7993

Test Plan

Tested together with next diff where this is used.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Fri, May 26, 1:17 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptFri, May 26, 1:17 AM
bartek added a child revision: D7995: [web] Fix thread gallery for encrypted media.Fri, May 26, 1:19 AM
Harbormaster completed remote builds in B19788: Diff 27084.Fri, May 26, 1:34 AM
bartek published this revision for review.Fri, May 26, 1:52 AM
atul requested changes to this revision.Fri, May 26, 8:44 AM

We should use the classnames component to conditionally apply styles on web. Added some examples of usage inline.

Definitely let me know if I'm missing something + feel free to re-request review

web/modals/threads/gallery/thread-settings-media-gallery-item.react.js
34–41 ↗(On Diff #27084)

Can we use the classnames utility instead of passing things to img style prop?

There should be a lot of examples of classnames usage throughout the codebase eg:

6b1ca2.png (432×2 px, 122 KB)

a3d6c8.png (492×2 px, 141 KB)

Definitely let me know if I'm missing something here though

This revision now requires changes to proceed.Fri, May 26, 8:44 AM
atul attached a referenced file: F560005: a3d6c8.png. (Show Details)Fri, May 26, 8:44 AM
atul attached a referenced file: F560004: 6b1ca2.png. (Show Details)
bartek added inline comments.Fri, May 26, 8:50 AM
web/modals/threads/gallery/thread-settings-media-gallery-item.react.js
34–41 ↗(On Diff #27084)

Ok but how do I pass dynamic URL value to such classname thing?

bartek requested review of this revision.Sun, May 28, 11:13 PM

Re-requesting because I see no way of creating a dynamic style with classnames

atul accepted this revision.Tue, May 30, 9:31 AM

Re-requesting because I see no way of creating a dynamic style with classnames

Sorry for blocking stack on this, should have read through more carefully

web/modals/threads/gallery/thread-settings-media-gallery-item.react.js
34–41 ↗(On Diff #27084)

Good point... not sure what I was thinking. Sorry for blocking stack on this.

This revision is now accepted and ready to land.Tue, May 30, 9:31 AM
Closed by commit rCOMMf599a6ea375d: [web] Add MediaGalleryItem component (authored by bartek). · Explain WhyTue, May 30, 11:46 PM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMMf599a6ea375d: [web] Add MediaGalleryItem component.

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
gallery/
71 lines

Diff 27289

View Options

web/modals/threads/gallery/thread-settings-media-gallery-item.react.js

Loading...