Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

Fixing the HtmlString labels in latest versions of PHP #142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nocodelab
Copy link
Contributor

With this pull request the package is now compatible with the latest versions of PHP 8.*.
The HTML labels were not rendered due to non-compatible type declarations in label-related methods.
Enjoy!

With this pull request the package is now compatible with the latest versions of PHP 8.*.
The HTML labels were not rendered due to non-compatible type declarations in label-related methods.
Enjoy!
@nocodelab nocodelab closed this Mar 24, 2022
@nocodelab nocodelab reopened this Mar 24, 2022
@nocodelab nocodelab closed this Mar 24, 2022
@nocodelab nocodelab reopened this Mar 24, 2022
@dwightwatson
Copy link
Owner

Isn't this new typehinting syntax only supported in PHP 8.1? I would need to bump up the supported versions if so.

@nocodelab
Copy link
Contributor Author

Ciao @dwightwatson, yes you are right.
Supported PHP version needs to be updated as well.

@nocodelab
Copy link
Contributor Author

Ciao @dwightwatson, I have updated the composer.json with the PHP 8.1 version bump.
Can you please merge it?

Thanks!

@dwightwatson
Copy link
Owner

Thanks for that. I'm still debating whether this is a change I want to make as it's quite dramatic.

I think we'd need 4 things in order to merge this:

  • Drop support for Laravel 8.x - if it's going to target PHP 8.1 we may as well forward-focus this,
  • Instead of HtmlString we should probably use the Htmlable interface instead,
  • The Htmlable interface needs to be imported into the file (HtmlString is not, so it wouldn't work as-is), and
  • It needs to test coverage for all affected methods to ensure they work with the new argument type.

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.

None yet

2 participants