JW Player #433

anoblet · 2024-09-11T12:45:22Z

https://app.clickup.com/t/3tnvw4y

github-actions · 2024-09-11T12:47:03Z

size-limit report 📦

Path	Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js	44.75 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js	14.98 KB (+26.02% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js	29.23 KB (0%)
packages/cxl-ui/pkg/dist-web/vendor.js	157.96 KB (+14.28% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-institute.js, packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js, packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js, packages/cxl-ui/pkg/dist-web/cxl-ui.js, packages/cxl-ui/pkg/dist-web/manifest.js, packages/cxl-ui/pkg/dist-web/unresolved.js, packages/cxl-ui/pkg/dist-web/vendor.js	291.35 KB (+4.97% 🔺)

pawelkmpt

You squashed commits, nice 👍

Please polish one thing in commit messages:

- [cxl-jw-player]
+ cxl-jw-player

Co-authored-by: Andrew Noblet <[email protected]> feat(cxl-ui): tweak jw player component Co-authored-by: Andrew Noblet <[email protected]> feat(cxl-ui): rename caption mixin to transcript mixin Co-authored-by: Andrew Noblet <[email protected]> feat(cxl-ui): tweak transcript mixin Co-authored-by: Andrew Noblet <[email protected]> feat(cxl-ui): rename chapter mixin to chapter navigation mixin Co-authored-by: Andrew Noblet <[email protected]> feat(cxl-ui): tweak chapter navigation mixin Co-authored-by: Andrew Noblet <[email protected]> feat(cxl-ui): add cxl-jw-player-transcript element Co-authored-by: Andrew Noblet <[email protected]> feat: cleanup, refactor & improve the code for PHP `cxl/jw-player` * jw-player height tweaks * [jw-player] add support for sources cherry-pick: #236 * cxl-jw-player set up api endpoint cherry-pick: #234

…d, librarySource feat(cxl-ui): cxl-jw-player add hidden property style feat(cxl-ui): cxl-jw-player set container display and height feat(cxl-ui): cxl-jw-player fix playlist story feat(cxl-ui): cxl-jw-player fix playlist styles feat(cxl-ui): cxl-jw-player add next up CTA feat(cxl-ui): cxl-jw-player hide default next up overlay feat(cxl-ui): cxl-jw-player add pointer events to the link not the container feat(cxl-ui): cxl-jw-player move next up styles to the global directory feat(cxl-ui): cxl-jw-player refactor private methods and properties to protected feat(cxl-ui): cxl-jw-player refactor for playlists feat(cxl-ui): cxl-jw-player move styles to global directory feat(cxl-ui): cxl-jw-player remove save position endpoint request feat(cxl-ui): cxl-jw-player add the ability to save the playback rate feat(cxl-ui): cxl-jw-player add the ability to play protected content feat(cxl-ui): cxl-jw-player fix media and playlist source url feat(cxl-ui): cxl-jw-player fix and improve next up feature feat(cxl-ui): cxl-jw-player remove api secret default value feat(cxl-ui): cxl-jw-player fix save position when using a playlist

feat(cxl-ui): cxl-jw-player fix chapter navigation when using playlists feat(cxl-ui): cxl-jw-player fix CTA pointer events feat(cxl-ui): cxl-jw-player fix saving position when using a playlist feat(cxl-ui): cxl-jw-player update component references feat(cxl-ui): cxl-jw-player fix poor refactoring

pawelkmpt · 2024-09-20T11:24:13Z

I managed to run a player on WPS. ~~It doesn't show transcript though, but I didn't dig into details yet as I'm coming with security thought.~~
Made transcript working

Component requires to pass api-secret which makes it a public value and it should not happen. Otherwise everyone can use it. We should rather sign and obtain necessary data on the backend side.

@lkraav @anoblet your thoughts?

EDIT
We have /wp-json/cxl/jwp/content-token REST API endpoint for signing resource ID. It was created for JW Player app. Maybe can be used here too?

packages/cxl-ui/src/components/cxl-jw-player/mixins/BaseMixin.js

packages/cxl-ui/src/components/cxl-jw-player/mixins/TranscriptMixin.js

lkraav · 2024-09-20T12:15:36Z

packages/cxl-ui/src/components/cxl-jw-player/mixins/index.js

+export { BaseMixin } from './BaseMixin';
+export { ChapterNavigationMixin } from './chapter-navigation/index.js';
+export { NextUpMixin } from './NextUpMixin';
+export { StateMixin } from './StateMixin';
+export { TranscriptMixin } from './TranscriptMixin';


"kebab-case everywhere" possible?

Maybe we can also install https://github.com/loeffel-io/ls-lint one day.

I'm trying to remember the reason for using pascal case vs. kebab case for mixins. I'll take a look and find a standard.

packages/cxl-ui/src/components/cxl-jw-player/mixins/TranscriptMixin.js

pawelkmpt · 2024-09-23T12:56:20Z

Component requires to pass api-secret which makes it a public value and it should not happen. Otherwise everyone can use it. We should rather sign and obtain necessary data on the backend side.

@lkraav @anoblet your thoughts?

EDIT We have /wp-json/cxl/jwp/content-token REST API endpoint for signing resource ID. It was created for JW Player app. Maybe can be used here too?

Maybe component should get signed URL as param, e.g. https://cdn.jwplayer.com/v2/media/fVRbGa56?sources=hls&token=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3MjcwOTk2MTQsInJlc291cmNlIjoidjIvbWVkaWEvZlZSYkdhNTYifQ.ZWjguN1Rya0rHXz-kH3TomHINFwp_-NA6oBcnDwrNAU and inside fetch data, process it and display what needs to be displayed?

Using REST API endpoint requires authentication too so some kind of secret would be still needed. With signing on the server side and providing signed URL we have this problem solved.

lkraav · 2024-09-23T13:23:44Z

Maybe

Undoubtedly there's documented best practices out there for this age old problem, I'd welcome a link to a worthy guide?

anoblet · 2024-09-23T13:39:09Z

Component requires to pass api-secret which makes it a public value and it should not happen. Otherwise everyone can use it. We should rather sign and obtain necessary data on the backend side.

We saw this as a potential issue. Instead of providing api-secret, along with library-id, and media-id. You can omit api-secret, and use library-source, and media-source instead, where the JWT signing happens on the backend, and the full URL is provided to the component.

pawelkmpt · 2024-09-23T14:50:42Z

You can omit api-secret, and use library-source, and media-source instead, where the JWT signing happens on the backend, and the full URL is provided to the component.

As API request or straighforward <cxl-jw-player url="<signed_url>">?

I personally would pass signed URL.

pawelkmpt · 2024-09-23T14:57:37Z

Undoubtedly there's documented best practices out there for this age old problem, I'd welcome a link to a worthy guide?

ChatGPT suggests using signing API: https://chatgpt.com/share/66f18132-43dc-8002-95ab-4f699f384976

As can seen above, I considered it too but managing REST API endpoint and authenticating request seems to be overengineering in our case when I can just output HTML with webcomponent having already signed URL. And cached URL

anoblet · 2024-09-23T15:05:41Z

As API request or straighforward ?

This would be the full signed URL. There are two different types of signed URL, JWT and non JWT, we would need them both: https://docs.jwplayer.com/platform/reference/protect-your-content-with-signed-urls

pawelkmpt · 2024-09-23T16:01:01Z

As API request or straighforward ?

This would be the full signed URL. There are two different types of signed URL, JWT and non JWT, we would need them both: https://docs.jwplayer.com/platform/reference/protect-your-content-with-signed-urls

Oh, indeed. Good catch @anoblet. I didn't see it before but now see JWT is used for a video and non JWP for library. So we'd need two arguments:

video-url or content-url
lib-url

or it's more complex?

Anyway, please adjust the component for the new approach. Save in new commits, do not overwrite existing just in case we'd like to get back to the past version @anoblet

anoblet · 2024-09-23T16:09:10Z

This is already supported with library-source, media-source, and playlist-source attributes. Sample code:

aybolit/packages/cxl-ui/src/components/cxl-jw-player/mixins/BaseMixin.js

Line 111 in 2a873b4

async _getMedia() {

https://app.clickup.com/t/apru1v?comment=90140063544642

pawelkmpt

2 more things on ClickUp

packages/cxl-ui/src/components/cxl-jw-player/index.html.js

pawelkmpt · 2024-10-01T10:37:55Z

packages/cxl-ui/src/components/cxl-jw-player/index.html.js

+                    <vaadin-checkbox
+                      @change=${this._toggleShouldScroll}
+                      ?checked=${this._shouldScroll}
+                      label="Scroll"
+                    ></vaadin-checkbox>


It's not visible. Is it needed?

pawelkmpt · 2024-10-01T13:53:26Z

@anoblet I have a suspicion CXLJWPlayerElement is compiled into cxl-ui-institue.js package. We also have separate cxl-ui-jwplayer.js and component should be imported only in the latter one.

anoblet · 2024-10-02T12:42:13Z

I have a suspicion CXLJWPlayerElement is compiled into cxl-ui-institue.js package. We also have separate cxl-ui-jwplayer.js and component should be imported only in the latter one.

I only see it imported in packages/cxl-ui/src/index-jwplayer.js and packages/cxl-ui/src/index-storybook.js.

pawelkmpt · 2024-10-02T13:49:42Z

I have a suspicion CXLJWPlayerElement is compiled into cxl-ui-institue.js package. We also have separate cxl-ui-jwplayer.js and component should be imported only in the latter one.

I only see it imported in packages/cxl-ui/src/index-jwplayer.js and packages/cxl-ui/src/index-storybook.js.

Continued here: https://app.clickup.com/t/apru1v?comment=90140065293863

pawelkmpt

Commit messages could be more precise

- feat(cxl-ui): cxl-jw-player add scroll to feature when pressing enter
+ feat(cxl-ui): cxl-jw-player transcript scroll to next result when pressing enter

- feat(cxl-ui): cxl-jw-player remove empty tracks
+ feat(cxl-ui): cxl-jw-player transcript remove empty tracks

packages/cxl-ui/src/components/cxl-jw-player/mixins/TranscriptMixin.js

…ssing enter https://app.clickup.com/t/apru1v?comment=90140065274148

https://app.clickup.com/t/apru1v?comment=90140065298953

https://app.clickup.com/t/apru1v?comment=90140065514261

…ed on https://app.clickup.com/t/apru1v?comment=90140063544642

…esult https://app.clickup.com/t/apru1v?comment=90140065274148&threadedComment=90140065514123

https://app.clickup.com/t/apru1v?comment=90140065293863

…lors 464df2f https://app.clickup.com/t/apru1v?comment=90140065274148&threadedComment=90140065750956

anoblet requested review from lkraav and pawelkmpt September 11, 2024 12:45

anoblet mentioned this pull request Sep 11, 2024

cxl jwplayer upgrades & tweaks #239

Closed

pawelkmpt reviewed Sep 11, 2024

View reviewed changes

saas786 and others added 6 commits September 11, 2024 13:13

feat(cxl-ui): cxl-jw-player add styles directory

c83003c

feat(cxl-ui): cxl-jw-player add ability to save playlist index

27b4a22

feat(cxl-ui): cxl-jw-player add nextupoffset attribute

ab8abe3

anoblet force-pushed the anoblet/feat/cxl-jw-player branch from 84f1ba6 to 702065a Compare September 11, 2024 13:15

pawelkmpt requested changes Sep 20, 2024

View reviewed changes

packages/cxl-ui/src/components/cxl-jw-player/mixins/BaseMixin.js Outdated Show resolved Hide resolved

packages/cxl-ui/src/components/cxl-jw-player/mixins/BaseMixin.js Outdated Show resolved Hide resolved

pawelkmpt reviewed Sep 20, 2024

View reviewed changes

packages/cxl-ui/src/components/cxl-jw-player/mixins/TranscriptMixin.js Show resolved Hide resolved

lkraav reviewed Sep 20, 2024

View reviewed changes

pawelkmpt reviewed Sep 20, 2024

View reviewed changes

packages/cxl-ui/src/components/cxl-jw-player/mixins/TranscriptMixin.js Show resolved Hide resolved

feat(cxl-ui): cxl-jw-player refactor base url

2a873b4

anoblet force-pushed the anoblet/feat/cxl-jw-player branch from a61f29b to 2a873b4 Compare September 23, 2024 14:31

feat(cxl-ui): cxl-jw-player remove jw player link

b72df62

https://app.clickup.com/t/apru1v?comment=90140063544642

anoblet force-pushed the anoblet/feat/cxl-jw-player branch from 018e719 to 6de900e Compare September 28, 2024 14:00

feat(cxl-ui): cxl-jw-player update player styles

9e0a30c

feat(cxl-ui): cxl-jw-player remove support for plugins

88913ef

anoblet force-pushed the anoblet/feat/cxl-jw-player branch from af50481 to 88913ef Compare September 28, 2024 15:39

anoblet added 4 commits September 28, 2024 16:17

feat(cxl-ui): cxl-jw-player update player styles

1ab4e12

feat(cxl-ui): cxl-jw-player update search result count styles

5bc46c2

fix(cxl-ui): cxl-jw-player scroll only the captions container

9e4fbd9

feat(cxl-ui): cxl-jw-player add error handling

f770f6c

pawelkmpt requested changes Oct 1, 2024

View reviewed changes

packages/cxl-ui/src/components/cxl-jw-player/index.html.js Outdated Show resolved Hide resolved

pawelkmpt reviewed Oct 1, 2024

View reviewed changes

anoblet added 2 commits October 1, 2024 13:10

fix(cxl-ui): cxl-jw-player reset play position on video complete

a4ae583

feat(cxl-ui): cxl-jw-player update result count styles

036bd53

anoblet force-pushed the anoblet/feat/cxl-jw-player branch 2 times, most recently from dc20c6a to 3143869 Compare October 2, 2024 14:16

pawelkmpt reviewed Oct 3, 2024

View reviewed changes

packages/cxl-ui/src/components/cxl-jw-player/mixins/TranscriptMixin.js Outdated Show resolved Hide resolved

packages/cxl-ui/src/components/cxl-jw-player/mixins/TranscriptMixin.js Outdated Show resolved Hide resolved

anoblet added 4 commits October 3, 2024 14:22

feat(cxl-ui): cxl-jw-player transcript scroll to next result when pre…

6d3c79f

…ssing enter https://app.clickup.com/t/apru1v?comment=90140065274148

feat(cxl-ui): cxl-jw-player transcript remove empty tracks

9df9201

https://app.clickup.com/t/apru1v?comment=90140065298953

feat(cxl-ui): cxl-jw-player add margin to the bottom of the captions

5639cbf

https://app.clickup.com/t/apru1v?comment=90140065514261

fix(cxl-ui): cxl-jw-player setup crashes when transcripts aren't turn…

e179515

…ed on https://app.clickup.com/t/apru1v?comment=90140063544642

anoblet force-pushed the anoblet/feat/cxl-jw-player branch from 0c3b937 to e179515 Compare October 3, 2024 14:23

feat(cxl-ui): cxl-jw-player transcript only highlight active search r…

464df2f

…esult https://app.clickup.com/t/apru1v?comment=90140065274148&threadedComment=90140065514123

anoblet force-pushed the anoblet/feat/cxl-jw-player branch from 6a6af4d to 464df2f Compare October 3, 2024 15:22

anoblet and others added 3 commits October 4, 2024 18:41

fix(cxl-ui): cxl-jw-player toggle captions not properly working

73371c3

fix(cxl-ui): cxl-course-dialog remove import of cxl-jw-player

dadc6bb

https://app.clickup.com/t/apru1v?comment=90140065293863

refactor(cxl-ui): cxl-jw-player modify transcript search highlight co…

da3dd1f

…lors 464df2f https://app.clickup.com/t/apru1v?comment=90140065274148&threadedComment=90140065750956

pawelkmpt approved these changes Oct 7, 2024

View reviewed changes

pawelkmpt merged commit 5622f37 into master Oct 7, 2024
5 checks passed

This was referenced Oct 7, 2024

JW Player #394

Closed

feat/anoblet/jw-player-gravity-form #217

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JW Player #433

JW Player #433

anoblet commented Sep 11, 2024 •

edited

Loading

github-actions bot commented Sep 11, 2024 •

edited

Loading

pawelkmpt left a comment

pawelkmpt commented Sep 20, 2024 •

edited

Loading

lkraav Sep 20, 2024 •

edited

Loading

anoblet Sep 23, 2024

pawelkmpt commented Sep 23, 2024

lkraav commented Sep 23, 2024

anoblet commented Sep 23, 2024 •

edited

Loading

pawelkmpt commented Sep 23, 2024 •

edited

Loading

pawelkmpt commented Sep 23, 2024

anoblet commented Sep 23, 2024 •

edited

Loading

pawelkmpt commented Sep 23, 2024

anoblet commented Sep 23, 2024

pawelkmpt left a comment

pawelkmpt Oct 1, 2024

pawelkmpt commented Oct 1, 2024

anoblet commented Oct 2, 2024

pawelkmpt commented Oct 2, 2024

pawelkmpt left a comment

JW Player #433

JW Player #433

Conversation

anoblet commented Sep 11, 2024 • edited Loading

github-actions bot commented Sep 11, 2024 • edited Loading

size-limit report 📦

pawelkmpt left a comment

Choose a reason for hiding this comment

pawelkmpt commented Sep 20, 2024 • edited Loading

lkraav Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

anoblet Sep 23, 2024

Choose a reason for hiding this comment

pawelkmpt commented Sep 23, 2024

lkraav commented Sep 23, 2024

anoblet commented Sep 23, 2024 • edited Loading

pawelkmpt commented Sep 23, 2024 • edited Loading

pawelkmpt commented Sep 23, 2024

anoblet commented Sep 23, 2024 • edited Loading

pawelkmpt commented Sep 23, 2024

anoblet commented Sep 23, 2024

pawelkmpt left a comment

Choose a reason for hiding this comment

pawelkmpt Oct 1, 2024

Choose a reason for hiding this comment

pawelkmpt commented Oct 1, 2024

anoblet commented Oct 2, 2024

pawelkmpt commented Oct 2, 2024

pawelkmpt left a comment

Choose a reason for hiding this comment

anoblet commented Sep 11, 2024 •

edited

Loading

github-actions bot commented Sep 11, 2024 •

edited

Loading

pawelkmpt commented Sep 20, 2024 •

edited

Loading

lkraav Sep 20, 2024 •

edited

Loading

anoblet commented Sep 23, 2024 •

edited

Loading

pawelkmpt commented Sep 23, 2024 •

edited

Loading

anoblet commented Sep 23, 2024 •

edited

Loading