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.

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

Isolate actions strategies in order to code improvement and programmatic usage. #749

Merged
merged 9 commits into from
May 18, 2021

Conversation

obetomuniz
Copy link
Collaborator

@obetomuniz obetomuniz commented Mar 29, 2021

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the dev branch for v2.x (or to a previous version branch), not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • New/updated tests are included

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

This is a proposal to modernize the library in order to support more features, scale the current features in a healthy way and give more flexibility to the user.

Copy link
Collaborator

@helderberto helderberto left a comment

Choose a reason for hiding this comment

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

IMHO, nothing is blocking, great work! 🚀

Copy link
Collaborator

@ktfth ktfth left a comment

Choose a reason for hiding this comment

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

Good work

@obetomuniz obetomuniz removed proposal wip When this flag is present: DON'T MERGE :) labels Apr 11, 2021
@obetomuniz obetomuniz changed the title Isolate cut and copy strategies in order to reuse and programmatic usage. Isolate actions strategies in order to code improvement and programmatic usage. Apr 15, 2021
Copy link
Collaborator

@vitormalencar vitormalencar left a comment

Choose a reason for hiding this comment

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

Nice work @obetomuniz, I just have one small question, does our TS type definitions need to be updated as well?

@obetomuniz
Copy link
Collaborator Author

Nice work @obetomuniz, I just have one small question, does our TS type definitions need to be updated as well?

That's definitely a thing to consider since we now have two new static methods being exposed. I will take a look on it today before merge. Tks man!

Copy link
Collaborator

@vitormalencar vitormalencar left a comment

Choose a reason for hiding this comment

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

Nice 👏

@obetomuniz obetomuniz merged commit 44df750 into master May 18, 2021
@howlger
Copy link

howlger commented Sep 20, 2021

In clipboard.min.js o.remove() (instead of o.parentNode.removeChild(o)) seems wrong to me, since Internet Explorer 11 does not support remove() (causing SCRIPT438: Object doesn't support property or method 'remove' error/exception).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants