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.

Sign up for GitHub

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

Jump to bottom

chore: Miscellaneous cleanups #126

Merged
merged 3 commits into from
Jul 4, 2023
Merged

chore: Miscellaneous cleanups #126

merged 3 commits into from
Jul 4, 2023

Conversation

phloxic
Copy link
Contributor

@phloxic phloxic commented Jun 5, 2023

No description provided.

@phloxic
Copy link
Contributor Author

phloxic commented Jun 5, 2023

@mister-ben - thanks for the plugin. Here are few minor things which I came across while working with it.

@mister-ben
Copy link
Owner

The changes to README and plugin.js are good, thanks for the update. However I see no need to change to using sass.

@phloxic
Copy link
Contributor Author

phloxic commented Jun 16, 2023

[...] However I see no need to change to using sass.

Fair enough. FYI, 2 practical reasons why I suggested it:

  1. I like to have an unminified CSS file to hand
  2. I intend to propose a semi-transparent background for the overlay; the color is nicked from Video.js' primary-background-color, and the reference to it easier to recognize with rgba(#2B333F, 0.6)

Do you prefer a new PR or is a force push to this branch without the last commit ok?

@phloxic
Copy link
Contributor Author

phloxic commented Jun 23, 2023

@mister-ben - I've removed the sass related stuff and rebased onto master.

@mister-ben mister-ben changed the title Miscellaneous cleanups chore: Miscellaneous cleanups Jul 4, 2023
@mister-ben mister-ben merged commit dcc9d89 into mister-ben:master Jul 4, 2023
@mister-ben
Copy link
Owner

Thanks. A background on the icons might be a good idea.

@phloxic phloxic deleted the misc-cleanup branch July 5, 2023 12:00
@phloxic phloxic mentioned this pull request Jul 8, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@phloxic @mister-ben