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

feat: Add RichTextVideo extension for video playback #735

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rfgamaral
Copy link
Member

Overview

This adds a Video extension to Typist, which is very similar to the one that we have for Images - but for Videos - and parts of the code (mostly related to the uploading mechanism) have been copy-pasted.

Although this was built to be used as an example for a presentation where I go through the steps of building a Typist extension, this could be very well considered ready for prime time. There might be a thing or two that we might want to change and/or do differently, but other than that, the code is pretty much production-ready. However, this will be a draft PR until we are sure that we want to include such extension in our products.

PR Checklist

Test plan

  • Open the preview Storybook deployed to Netlify
  • Open the Rich-text → Default story
    • Smoke test the Video extension and make sure it works as you'd expect it to

@rfgamaral rfgamaral self-assigned this Apr 11, 2024
Copy link

netlify bot commented Apr 11, 2024

Deploy Preview for doist-typist ready!

Name Link
🔨 Latest commit fac9454
🔍 Latest deploy log https://app.netlify.com/sites/doist-typist/deploys/6728f2559c37e80008603747
😎 Deploy Preview https://deploy-preview-735--doist-typist.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines +340 to +343
if (this.options.video !== false) {
extensions.push(RichTextVideo.configure(this.options?.video))
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this is something that we want to be added to all our editors by default, forcing us to disable this extension for the ones that we don't want to provide support for (e.g., Todoist task name, where we already disable images, along with many others).

The alternative is to remove this from RichTextKit, export it, and let consumers include the extension or not. In other words, make this an optional "bonus" extension.

@rfgamaral rfgamaral force-pushed the ricardo/video-extension branch from d5325b9 to c1b6d97 Compare November 4, 2024 16:06
@rfgamaral rfgamaral force-pushed the ricardo/video-extension branch from c1b6d97 to fac9454 Compare November 4, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant