Skip to content
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

Closed
18 of 20 tasks
francoisl opened this issue Jun 8, 2023 · 57 comments
Closed
18 of 20 tasks

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

francoisl opened this issue Jun 8, 2023 · 57 comments
Assignees
Labels
Monthly KSv2 NewFeature Something to build that is a new item. Not a priority Planning Changes still in the thought process Reviewing Has a PR in review

Comments

@francoisl
Copy link
Contributor

francoisl commented Jun 8, 2023

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

  • Post Proposal (full Problem/Solution statement) in #expensify-open-source
  • Wait at least one full business day, and until the post has a majority (2/3) of positive reactions (👍)
  • Paste Proposal in the space above with a link to the Slack thread
  • Email strategy@expensify.com and paste in the Proposal
  • Fill out the High-level overview of the problem, Timeline, and Terminology sections of the Design Doc
  • Email strategy@expensify.com (continue the same email chain as before) with the link to your Design Doc
  • Host a pre-design meeting (example) in #expensify-open-source to discuss any necessary details in public before filling out the High-level of proposed solution section.
  • Fill out the High-level of proposed solution section
  • Email stategy@expensify.com again with links to the doc and pre-design conversation in Slack
  • Add the DesignDocReview label to get the High-level of proposed solution section reviewed
  • Respond to any questions or concerns and bring up blockers in Slack to get a consensus if necessary
  • Confirm that the doc has the minimum necessary number of reviews before proceeding
  • Host another pre-design meeting in #expensify-open-source to ask for engineering feedback on the technical solution.
  • Fill out the Detailed implementation of the solution and related sections.
  • Re-add the DesignDocReview label to this issue
  • Respond to any questions or concerns and bring up blockers in Slack to get consensus if necessary
  • Confirm that the doc has the minimum necessary number of reviews before proceeding
  • Email strategy@expensify.com one last time to let them know the Design Doc is moving into the implementation phase
  • Implement the changes
  • Send out a follow up email to strategy@expensify.com once everything has been implemented and do a Project Wrap-Up retrospective that provides:
    • Summary of what we accomplished with this project
    • What went well?
    • What could we have done better?
    • What did we learn?
@francoisl francoisl added Daily KSv2 NewFeature Something to build that is a new item. labels Jun 8, 2023
@francoisl francoisl self-assigned this Jun 8, 2023
@melvin-bot

This comment was marked as outdated.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jun 8, 2023
@francoisl
Copy link
Contributor Author

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

@francoisl francoisl added the Planning Changes still in the thought process label Jun 8, 2023
@jczekalski
Copy link
Contributor

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

@francoisl
Copy link
Contributor Author

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

@melvin-bot melvin-bot bot removed the Overdue label Jun 17, 2023
@melvin-bot melvin-bot bot added the Overdue label Jun 26, 2023
@puneetlath
Copy link
Contributor

puneetlath commented Jun 26, 2023

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@melvin-bot melvin-bot bot removed the Overdue label Jul 20, 2023
@jczekalski
Copy link
Contributor

jczekalski commented Jul 20, 2023

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
Copy link
Contributor Author

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?

@melvin-bot melvin-bot bot added the Overdue label Aug 2, 2023
@francoisl
Copy link
Contributor Author

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

@melvin-bot melvin-bot bot removed the Overdue label Aug 3, 2023
@jczekalski
Copy link
Contributor

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.

@melvin-bot melvin-bot bot added the Overdue label Jan 9, 2024
@francoisl francoisl added the Reviewing Has a PR in review label Jan 9, 2024
@melvin-bot melvin-bot bot removed the Overdue label Jan 9, 2024
@Skalakid Skalakid mentioned this issue Jan 24, 2024
58 tasks
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Monthly KSv2 labels Jan 24, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jan 26, 2024
@kowczarz
Copy link
Contributor

kowczarz commented Feb 6, 2024

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:

  • src/components/Attachments/AttachmentView/AttachmentViewVideo/index.js
  • src/components/HTMLEngineProvider/HTMLRenderers/VideoRenderer.js
  • src/components/VideoPlayer/BaseVideoPlayer.js
  • src/components/VideoPlayer/IconButton.js
  • src/components/VideoPlayer/VideoPlayerControls/ProgressBar/index.js
  • src/components/VideoPlayer/VideoPlayerControls/VolumeButton/index.js
  • src/components/VideoPlayer/VideoPlayerControls/index.js
  • src/components/VideoPlayer/index.js
  • src/components/VideoPlayer/index.native.js
  • src/components/VideoPlayer/propTypes.js
  • src/components/VideoPlayer/utils.ts
  • src/components/VideoPlayerContexts/PlaybackContext.js
  • src/components/VideoPlayerContexts/VideoPopoverMenuContext.js
  • src/components/VideoPlayerContexts/VolumeContext.js
  • src/components/VideoPlayerPreview/VideoPlayerThumbnail.js
  • src/components/VideoPlayerPreview/index.js
  • src/components/VideoPopoverMenu/index.js
  • src/hooks/useThumbnailDimensions.ts
  • src/libs/API/parameters/UpdateWelcomeMessageParams.ts
  • src/pages/ReportWelcomeMessagePage.tsx
  • src/pages/ShareCodePage.js
  • src/pages/home/report/ReportDetailsShareCodePage.js
  • src/pages/settings/Report/NotificationPreferencePage.js
  • src/pages/settings/Report/WriteCapabilityPage.js

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Feb 19, 2024
Copy link

melvin-bot bot commented Feb 19, 2024

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
Copy link
Contributor Author

The video player is on staging now!

Centralizing issues and follow-up tasks in this issue.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 20, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 23, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Feb 23, 2024

⚠️ 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.

Copy link

melvin-bot bot commented Apr 17, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Monthly KSv2 NewFeature Something to build that is a new item. Not a priority Planning Changes still in the thought process Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests