8000 Core - Make reset CSS configurable by joaopaulovieira · Pull Request #44 · clappr/clappr-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Core - Make reset CSS configurable #44

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
Merged
merged 10 commits into from
Nov 25, 2020
Merged

Core - Make reset CSS configurable #44

merged 10 commits into from
Nov 25, 2020

Conversation

joaopaulovieira
Copy link
Member

Summary

This PR creates the possibility to don't use nested-reset CSS mixin when the new option resetStyle is configured with the false value. One default value for resetStyle option is created to maintain the current behavior.

Also, the style injection is changed to allow the new feature.

The path pattern to access modules is updated to use the src alias on all files where is necessary to update the style injection.

Changes

  • rollup.config.js:
    • Config the rollup-plugin-postcss to enable inject style on the element itself instad the head page
  • player.js:
    • Create default value (true) for resetStyle option
  • core/public/style.scss:
    • Remove nested-reset CSS mixin
  • core/public/optional.scss:
    • Add nested-reset CSS mixin
  • core.js:
    • Add optional.scss accordingly the resetStyle value
  • core.js/container.js/html5_video.js/html_img.js/no_op.js:
    • Use src path alias to import modules;
    • Inject style on the element itself instead the head page;

How to test

  • Run public page;
  • Check the head tag on the page;
    • Should not have any anonymous style of any player component;
  • Check any player element;
    • Should have the style related on the same level;
  • Check core element on the page;
    • Should have two style elements (CoreStyle and OptionalStyle);
  • Set resetStyle option with the false value;
    • Should have only one style element (CoreStyle);

A picture tells a thousand words

Before this PR

Screen Shot 2020-11-10 at 16 03 08

Screen Shot 2020-11-10 at 16 03 35

After this PR

Screen Shot 2020-11-10 at 16 00 22

With resetStyle = true

Screen Shot 2020-11-10 at 16 00 46

With resetStyle = false

Screen Shot 2020-11-10 at 16 01 24

@joaopaulovieira joaopaulovieira added the enhancement New feature or request label Nov 10, 2020
hound[bot]
hound bot reviewed Nov 25, 2020
View reviewed changes
src/components/core/public/optional_reset.scss
@@ -0,0 +1,5 @@
@import 'reset';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings
8000

@jhonatangcavalcanti jhonatangcavalcanti merged commit 4589f91 into master Nov 25, 2020
@jhonatangcavalcanti jhonatangcavalcanti deleted the update-style-injection branch November 25, 2020 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hound hound[bot] hound[bot] left review comments

@leandromoreira leandromoreira leandromoreira approved these changes

@jhonatangcavalcanti jhonatangcavalcanti jhonatangcavalcanti approved these changes

@pedrochamberlain pedrochamberlain pedrochamberlain approved these changes

@towerz towerz Awaiting requested review from towerz

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@joaopaulovieira @leandromoreira @jhonatangcavalcanti @pedrochamberlain
0