-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: created thumbnail using expo-video-thumbnail #5893
Conversation
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.
Almost there.
Lint is failing.
}, | ||
playerIcon: { | ||
position: 'absolute', | ||
shadowColor: '#000', |
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.
Don't use fixed colors. Everything should come from themes.
flex: 1 | ||
}, | ||
image: { | ||
width: '100%', |
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.
Can you use flex?
encrypted?: boolean; | ||
}; | ||
|
||
const Thumbnail = ({ url, encrypted = false }: ThumbnailProps) => { |
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.
You asked my opinion about splitting Thumbnail in a separate file.
I said we should either keep on Video file (it's a small file, so it's ok) or we create a Video folder so message/Components/Attachments
is organized with one file per feature.
You create Thumbnail
file without a Video folder, so Attachments now have
- CollapsibleQuote (folder)
- Image (folder)
- AttachedActions.tsx
- Attachments.tsx
- Audio.tsx
- Reply.tsx
- Video.tsx
- Thumbnail.tsx (this one is only related to Video)
4a2db10
to
073cf34
Compare
233fa4c
to
2df934b
Compare
isReply?: boolean; | ||
msg?: string; | ||
} | ||
type Image = string | null; |
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.
type Image = string | null; | |
type TThumbnailImage = string | null; |
fontSize: 12 | ||
playerIcon: { | ||
position: 'absolute', | ||
shadowColor: themes.light.backdropColor, |
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.
light?
playerIcon: { | ||
position: 'absolute', | ||
shadowColor: themes.light.backdropColor, | ||
shadowOpacity: 0.3, |
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.
use theme
35e3dff
to
300d9f2
Compare
300d9f2
to
411d7ff
Compare
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.
One last missing change requested and you can merge it.
Proposed changes
Create a thumbnail to improve the video component UX.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-226
How to test or reproduce
Basic
Others
Screenshots
IOS:
Before:
videoIosBefore.mov
After:
Gravacao.de.Tela.2024-10-02.as.17.52.36.mov
Android:
Before:
android_before.webm
After:
videoPrAndroid.webm
Types of changes
Checklist
Further comments