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

Proposal for plyr-react component update #678

Closed
realamirhe opened this issue Nov 3, 2021 · 3 comments
Closed

Proposal for plyr-react component update #678

realamirhe opened this issue Nov 3, 2021 · 3 comments
Assignees
Labels
⚠️ Changes requested Pull Request needs changes before it can be reviewed again enhancement New feature or request feature_request help wanted Extra attention is needed

Comments

@realamirhe
Copy link
Collaborator

realamirhe commented Nov 3, 2021

By the latest version, we are using a fully integrated component using react-aptor as connector level connecting native video element to the plyr instance class. I was thinking about is there a way that we can get developers to access to being synced with the first render phase of our app.

Although there is a simple solution of getting onFirstRender and configuring that in the new hook, that is not so flexible.

The second idea is to pass the hole hook and make developers call it on their own. Imagine there is a hook that can usePlyr
which gets some arguments and returns a ref which must be bound to the video element and that's it.

const MyCustomPlyrComponent = () => {
 const ref = usePlyr(/* some arguments */)
 
 useEffect(loggerEffectOnFirstRender, [])
 
 return <video ref={ref} />
}

The package needs a tiny change for achieving this behavior and we can use the exact same mechanism for the default Plyr component.

export default Plyr
export const usePlyr
// export other needed things

Further info

The issue face has been reported here by @mnervik

@realamirhe realamirhe added enhancement New feature or request help wanted Extra attention is needed ⚠️ Changes requested Pull Request needs changes before it can be reviewed again feature_request labels Nov 3, 2021
@realamirhe realamirhe pinned this issue Nov 3, 2021
@realamirhe
Copy link
Collaborator Author

hey @chintan9, hope everything is going well for you.

Are you going to publish the latest master version with a bug-fix patch, cause it is still an old version?

There is also a possible way to update the readme by adding the alpha version directly but it seems unprofessional and we didn't give feedback from users for a while (#654, #605, #562, #511, #502, #474#, #339)

// latest alpha version
yarn add git://github.com/chintan9/plyr-react.git#package

@realamirhe
Copy link
Collaborator Author

realamirhe commented Jan 17, 2022

Hello everybody

I had a session with @bradrice And did some experiments on the proposed method
Here (dev/example) I add a new method called usePlyr and a custom audio-player example that can easily bind the.on method and so on...

Wait for your feedbacks 😊

@realamirhe
Copy link
Collaborator Author

realamirhe commented Jan 25, 2022

Custom Components need to be added

@chintan9 chintan9 closed this as completed May 2, 2022
@chintan9 chintan9 unpinned this issue Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ Changes requested Pull Request needs changes before it can be reviewed again enhancement New feature or request feature_request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants