Refactor the <Multimedia> component to be a function component. This will make managing further thumbhash logic much easier.
Details
Multimedia are still displayed as before (both in chat and input bar). Clicking on the message opens modal. Remove button in input bar works. URL revoking is tested by console.log and ensuring it's called once when the message is sent.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
web/media/multimedia.react.js | ||
---|---|---|
84–96 ↗ | (On Diff #26726) | It appears that this code has changed... previously it was just passing mediaSource to media. Two quick notes:
|
93 ↗ | (On Diff #26726) | Wondering why this invariant is necessary... based on Flow types, it appears that uri should always be set (but perhaps it can be an empty string?) |
155 ↗ | (On Diff #26726) | Wondering why you replaced the spread with explicit props. Is it for readability? Or does a future diff add a prop or something? |
191 ↗ | (On Diff #26726) | This might be a good component to wrap with a React.memo... I assume the performance costs of rerendering media are non-negligible |
web/media/multimedia.react.js | ||
---|---|---|
84–96 ↗ | (On Diff #26726) | The logic stays the same, it just makes better ground for future diffs, but you're right this could have been possibly done in one of the next diffs. This is reverted anyway in D7947
Ok, makes sense |
93 ↗ | (On Diff #26726) |
This invariant is for Flow which for some reason isn't able to see that we're already excluded EncryptedMediaType media sources, and for MediaType sources uri is always present. Anyway, I update the MultimediaModal props to match mediaSource type again so this change will be reverted. |