search

bastislack / highline-freestyle

Webapp for Highline Freestyle
GNU General Public License v3.0
10 stars 9 forks source link

Create central VideoEmbed component #256

Closed JulianDietzel closed 1 year ago

JulianDietzel commented 1 year ago

Addresses #250, creating a centralized component for embedded videos to avoid duplicate code.

Note: There is a change to the webpack config. This change is only a small one for webpack to also recognize ".jsx" files.

For the future it might be easier to use a module like the react-social-media-embed one to embed videos. For now the refactor already improves the situation enough though.

JulianDietzel commented 1 year ago

Yes, I think renaming the appropriate files to .jsx would be good. It would make it easier to tell Components and just plain JS files apart. Also, making this changes should take less than 15 minutes, taking very little work.

And yes, this does get obsoleted by switching over to TypeScript, but again, as it takes very little time to make this change I think it is worth it for the meantime. Additionally, when switching to TypeScript, js files should become ts files and jsx files should become tsx files. So making the distinction now would probably be helpful when switching over to TS as well.

JulianDietzel commented 1 year ago

@bastislack What do you think? Should wait with the change of the file endings for the switch over to TS or do it now? Or, more specifically for this context: Should I keep the file endings of this commit the way they are or stick with the current standard and just rename the them to .js?

bastislack commented 1 year ago

I guess we can just leave it as it is for now :)