search

S4-NetQuest / react-scorm-provider

Components to easily enable SCORM API communication in React projects.
MIT License
61 stars 19 forks source link

Do you want this fork as PR ? #4

Closed Sangrene closed 4 years ago

Sangrene commented 4 years ago

Hi,

I recently forked this repo to include Typescript typings and a fallback to localStorage if the module is not used in a LMS. Are these features in the scope of this library and if so, do you want me to open a PR ?

JayV30 commented 4 years ago

Hi my friend! I think these changes fall outside the scope of this project.

re: fallback to localStorage - I think this is a task that should be left to the developer on a per-project basis. Obviously localStorage does not persist to the LMS and could lead to many issues. As an alternative to not making a connection to the SCORM API, it is certainly an option for local persistence, but outside the scope of this simple HOC.

re: typescript - I'm not opposed to Typescript in any way, but not familiar enough with it yet to want to include in this project. This is a result of my own limitations in not yet learning Typescript!

I also took a quick a look at your fork and I think there's lots of cool stuff you are implementing, but I don't currently have time to expand this project or review PRs. I'm certainly very open to collaboration, but again don't have time now to work with anyone on a roadmap. I don't want to implement more changes without working out some direction and goals.

I think it's AWESOME that you expanding this and maybe when we both have time to discuss in further detail we could collaborate?

Sangrene commented 4 years ago

Hi Jason,

Actually after playing with my fork for a bit, I ended up with the conclusion that I'm more comfortable decoupling the SCORM logic with React, placing it in a service and then using it in a clean architecture way.

Feel free to use any part of my fork though !