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

Reverse logic of html5() Method #43

Merged
merged 1 commit into from
Aug 11, 2024
Merged

Reverse logic of html5() Method #43

merged 1 commit into from
Aug 11, 2024

Conversation

jakejackson1
Copy link
Member

@jakejackson1 jakejackson1 commented Dec 11, 2022

Resolves issue where calling $qp('Content')->html5() would return an empty string, and calling $qp('Content')->html5('') would return Content. The correct logic, per ->html(), is to reverse how this method works.

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

If you call html5() without a parameter it will return an empty string, instead of the currently loaded document.

What is the new behavior?

Calling html5() without a parameter will now correctly return the loaded document, and calling html5('markup') will correctly set the innerHTML of the currently selected Element.

Does this introduce a breaking change?

  • Yes
  • No

Technically, if users relied on the previous behavior this would be considered a breaking change. But the previous behavior is functionally incorrect. It should act just like ->html(), but using the HTML5() class.

Other information

TODO: Unit test

@codecov
Copy link

codecov bot commented Dec 11, 2022

Codecov Report

Merging #43 (9b36844) into main (4280f56) will not change coverage.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               main      #43   +/-   ##
=========================================
  Coverage     88.40%   88.40%           
  Complexity     1349     1349           
=========================================
  Files            26       26           
  Lines          3088     3088           
=========================================
  Hits           2730     2730           
  Misses          358      358           
Impacted Files Coverage Δ
src/DOMQuery.php 82.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link

@cygro cygro left a comment

Choose a reason for hiding this comment

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

Approved pull request as this bug breaks the html5() method called without a markup parameter in PHP 8.3 (null as argument in several functions are deprecated).

Resolves issue where calling $qp('<strong>Content</strong>')->html5() would return an empty string, and calling $qp('<strong>Content</strong>')->html5('') would return <strong>Content</strong>. The correct logic, per ->html(), is to reverse how this method works.
@jakejackson1 jakejackson1 marked this pull request as ready for review August 11, 2024 23:42
@jakejackson1 jakejackson1 merged commit 9e9584c into main Aug 11, 2024
10 checks passed
@jakejackson1 jakejackson1 deleted the html5-fix branch August 11, 2024 23:45
@jakejackson1
Copy link
Member Author

@cygro thanks for your interest in having this problem fixed. I've written unit tests to verify the functionality and merged it into the codebase.

I ended up releasing this as a major update on account of the functionality changes, and you can install v4.0.0 👍 Enjoy.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants