Skip to content

Conversation

@talohana
Copy link

What: Added option to override hardcoded CodePen dimensions, with option to merge provided options with default options

Why: In order to change hardcoded CodePen's iframe width/height

How:

Each transformer can optionally export a const defaultOptions.
When the plugin runs it merges provided options ( services[name]) if exist with defaultOptions with precedence of provided options, and passes them as a second argument to the transformer exported getHTML function

Checklist:

  • Documentation
  • Tests
  • Ready to be merged

@codecov
Copy link

codecov bot commented Oct 15, 2020

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (668d719) to head (4d96646).
Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #146   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          300       300           
  Branches        95        96    +1     
=========================================
  Hits           300       300           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MichaelDeBoey MichaelDeBoey changed the title Adjust codepen dimensions feat(CodePen): Add support for height & width options Oct 16, 2020
@MichaelDeBoey MichaelDeBoey self-assigned this Oct 16, 2020
Copy link
Owner

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

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

Hi @talohana! 👋

Thanks for taking the time to implement this feature. 👊

I would however take a look at how the Twitch options are implemented and make them have a default value.
Exposing a defaultOptions value to merge them with the possible given options and pass that object back into the getHTML function is adding unnecessary imo.

@talohana
Copy link
Author

Hi @talohana! 👋

Thanks for taking the time to implement this feature. 👊

I would however take a look at how the Twitch options are implemented and make them have a default value.
Exposing a defaultOptions value to merge them with the possible given options and pass that object back into the getHTML function is adding unnecessary imo.

I see, it does look simpler, should I make the changes in this PR or create a new one?

@MichaelDeBoey
Copy link
Owner

MichaelDeBoey commented Oct 16, 2020

should I make the changes in this PR or create a new one?

@talohana You can just hard reset this branch onto upstream/master and push new commits

@talohana talohana closed this Oct 16, 2020
@talohana talohana force-pushed the adjust-codepen-dimensions branch from 7fd00c4 to 94e0a3e Compare October 16, 2020 20:01
@MichaelDeBoey MichaelDeBoey added the ⚙️ CodePen Issue or pull request regarding CodePen label Oct 16, 2020
@talohana talohana reopened this Oct 16, 2020
@hazem3500
Copy link

will this pull request be merged soon?

@talohana
Copy link
Author

will this pull request be merged soon?

I can work on it later today, @MichaelDeBoey any changes required rather than resolving the conflicts?

@talohana talohana force-pushed the adjust-codepen-dimensions branch from e90eb17 to fbac3f9 Compare February 20, 2021 15:01
@MichaelDeBoey MichaelDeBoey force-pushed the adjust-codepen-dimensions branch from fbac3f9 to 4d96646 Compare March 3, 2022 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚙️ CodePen Issue or pull request regarding CodePen

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants