-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Tracking] Add a Video Player to New Expensify #20471
Comments
This comment was marked as outdated.
This comment was marked as outdated.
@jczekalski FYI. GH won't let me co-assign you for some reason, feel free to co-assign yourself to this issue. |
@francoisl I can't assign myself. Perhaps you'll be able to co-assign me once I've commented in the issue? |
EOW update: Jan has been partially OOO this week, and I didn't have much time to update the doc. Hoping to resume working on this next week! 💪 |
Would this video player also work for audio files (e.g. mp3)? I'm assuming yes, because I'm thinking of it as a "media player" rather than "video player" but let me know if that's incorrect! |
@puneetlath this was brought up during the second pre-design (here), and Jan recommended we focus on video for now, since supporting audio files would possibly require different libs. |
Getting back to this, opened a Design issue to get started on the mockups. Will work on the high-level design doc this week. |
Hey @francoisl, I've already filled out some sections of the design doc, though it's still WIP and I'm working on a copy. Should I move everything to the actual design doc? |
Good to hear, thanks! For now it's fine, you can keep it in your WIP copy. One question though - did you figure out if we'll need to make backend changes to generate and store thumbnails – or if we'll be able to make the thumbnails directly from the client? |
Waiting on the mockups, in the meantime I remade a new design doc with the new template, and will continue filling it out. |
Hey @francoisl, I recently started working on a different project at Software Mansion and this issue was transferred to a different developer. I'll ask him to comment in the issue, so you can assign him. |
Since the Video Player feature is a blocker for other tasks and most of the code was written before introduction of TS restrictions, we decided to add new JS files to the main. As soon as we will merge #30829, we need to migrate following files to TS:
|
This issue has not been updated in over 15 days. @francoisl, @Skalakid eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
The video player is on staging now! Centralizing issues and follow-up tasks in this issue. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
@francoisl, @Skalakid, this Monthly task hasn't been acted upon in 6 weeks; closing. If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead. |
Proposal
Original Proposal
WN Internal Proposal
Project
Proposal: Add a Video Player to NewDot
Problem
At the moment, when a video is sent as a message, it is treated as a generic file attachment. This means that one has to first download it to their device, and then open it separately, out of the app.
Not only does this provide a subpar user experience, it's also inconsistent with images, which are displayed directly in the chat.
Solution
Add the ability for NewDot to play videos in-app, by implementing a video player.
Design Doc
https://docs.google.com/document/d/1Fh5Nu3D0-VW7xuV9Qc-OWWZl3-W5azx7Rr1lzNw4UGo/edit
Pre-designs
Pre-design 1
Pre-design 2
Tasks
#expensify-open-source
[email protected]
and paste in the Proposal[email protected]
(continue the same email chain as before) with the link to your Design Doc#expensify-open-source
to discuss any necessary details in public before filling out the High-level of proposed solution section.[email protected]
again with links to the doc and pre-design conversation in SlackDesignDocReview
label to get the High-level of proposed solution section reviewed#expensify-open-source
to ask for engineering feedback on the technical solution.DesignDocReview
label to this issue[email protected]
one last time to let them know the Design Doc is moving into the implementation phase[email protected]
once everything has been implemented and do a Project Wrap-Up retrospective that provides:The text was updated successfully, but these errors were encountered: