search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.49k stars 2.85k forks source link

[Tracking] Add a Video Player to New Expensify #20471

Closed francoisl closed 6 months ago

francoisl commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @abekkala (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

francoisl commented 1 year ago

@jczekalski FYI. GH won't let me co-assign you for some reason, feel free to co-assign yourself to this issue.

jczekalski commented 1 year ago

@francoisl I can't assign myself. Perhaps you'll be able to co-assign me once I've commented in the issue?

francoisl commented 1 year ago

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! 💪

puneetlath commented 1 year ago

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!

francoisl commented 1 year ago

@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.

francoisl commented 1 year ago

Getting back to this, opened a Design issue to get started on the mockups. Will work on the high-level design doc this week.

jczekalski commented 1 year ago

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?

francoisl commented 1 year ago

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?

francoisl commented 1 year ago

Waiting on the mockups, in the meantime I remade a new design doc with the new template, and will continue filling it out.

jczekalski commented 1 year ago

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.

Skalakid commented 1 year ago

Hey @francoisl, recently I've taken this task from Jan and started working on it. For now, I was creating my own POC where I was testing available packages and investigating challenges connected to them. Also, I continued writing Jan's design doc copy. I think we are having most of the information about the video player so I moved everything to the main file so you can take a look :D

francoisl commented 1 year ago

The doc is making good progress thanks to Michał. There's one open question regarding thumbnails that we took to Slack, other than that the doc is almost ready for review.

melvin-bot[bot] commented 1 year ago

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!

dannymcclain commented 1 year ago

It's not overdue Melvin. I posted an update to this issue without realizing not everyone had access to it.

Here's where we settled for the video player UI. ⏯️ image

You can find these screens (and all the little pieces for the player UI) in the NewDot / Video Player Figma file.

For the "more" 3-dot menu, we want to use our standard context menu styling.

image
francoisl commented 1 year ago

High-level design doc sent out for review, going to apply the DesignDocReview label to get some reviewers assigned.

https://docs.google.com/document/d/1Fh5Nu3D0-VW7xuV9Qc-OWWZl3-W5azx7Rr1lzNw4UGo/edit

melvin-bot[bot] commented 1 year ago

:wave: Hello Generalist Track Team - you have been assigned to review this High Level Design Doc. Please, treat this as a priority. Review this Stack Overflow for some tips on reviewing a design doc. Once you are done, simply press the Add "Reviewed Doc" comment button in the right hand side K2 pannel or follow these instructions.

puneetlath commented 1 year ago

I have read and reviewed this Design Doc!

roryabraham commented 1 year ago

I have read and reviewed this Design Doc!

tylerkaraszewski commented 1 year ago

I have read and reviewed this Design Doc!

Julesssss commented 1 year ago

I have read and reviewed this Design Doc!

techievivek commented 1 year ago

I have read and reviewed this Design Doc!

nkuoch commented 1 year ago

I have read and reviewed this Design Doc!

sakluger commented 1 year ago

I have read and reviewed this Design Doc!

danielrvidal commented 1 year ago

I have read and reviewed this Design Doc!

JmillsExpensify commented 1 year ago

I have read and reviewed this Design Doc!

heyjennahay commented 1 year ago

I have read and reviewed this Design Doc!

ryanschaffer commented 1 year ago

I have read and reviewed this Design Doc!

francoisl commented 12 months ago

Front-end part of the detailed section is mostly done, going to work on the backend.

francoisl commented 11 months ago

Detailed section of the doc is ready for review, going to reapply the DesignDoc label to get reviewers assigned.

melvin-bot[bot] commented 11 months ago

:wave: Hello Generalist Track Team - you have been assigned to review this Detailed Design Doc. Check out this Stack Overflow for some tips on reviewing a design doc. Once you are done, simply press the Add "Reviewed Doc" comment button in the right hand side K2 panel or follow these instructions.

puneetlath commented 11 months ago

I have read and reviewed this Design Doc!

tylerkaraszewski commented 11 months ago

I have read and reviewed this Design Doc!

joelbettner commented 11 months ago

I have read and reviewed this Design Doc!

joelbettner commented 11 months ago

I have read and reviewed this Design Doc!

quinthar commented 11 months ago

I have read and reviewed this Design Doc!

danielrvidal commented 11 months ago

I have read and reviewed this Design Doc!

nkuoch commented 11 months ago

I have read and reviewed this Design Doc!

laurenreidexpensify commented 11 months ago

I have read and reviewed this Design Doc!

iwiznia commented 11 months ago

I have read and reviewed this Design Doc!

francoisl commented 10 months ago

The project board is up with the issues, update here in Slack: https://expensify.slack.com/archives/C05NTGSDSF2/p1702093797797469

kowczarz commented 8 months ago

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 https://github.com/Expensify/App/pull/30829, we need to migrate following files to TS:

melvin-bot[bot] commented 8 months ago

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!

francoisl commented 8 months ago

The video player is on staging now!

Centralizing issues and follow-up tasks in this issue.

melvin-bot[bot] commented 8 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.

melvin-bot[bot] commented 8 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.

melvin-bot[bot] commented 8 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.

melvin-bot[bot] commented 8 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.

melvin-bot[bot] commented 8 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.

melvin-bot[bot] commented 8 months ago

⚠️ Looks like this issue was linked to a Deploy Blocker here

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.