Remove laravelcollective/html dependency and add Laravel 11/12 support by johnnyhawley · Pull Request #10 · akaunting/laravel-menu

johnnyhawley · 2026-02-13T01:31:03Z

Summary

This PR removes the dependency on laravelcollective/html package by implementing a custom HTML attribute rendering method, and extends support to Laravel 11 and 12 with PHP 8.3 and 8.4.

Key Changes

Removed laravelcollective/html dependency:
- Removed use Collective\Html\HtmlFacade as HTML; import from MenuItem.php
- Removed laravelcollective/html from composer.json requirements
- Removed HTML service provider registration from Provider.php
- Removed HTML service provider from test setup
Implemented custom HTML attribute rendering:
- Added renderHtmlAttributes() protected method to MenuItem.php that builds HTML attribute strings from arrays
- Updated getAttributes() method to use the new custom implementation instead of HTML::attributes()
- Updated return type documentation from mixed to string
Extended Laravel and PHP version support:
- Updated composer.json to support Laravel 11 and 12 (in addition to 9 and 10)
- Added PHP 8.3 and 8.4 to test matrix
- Updated testbench to support versions 9 and 10
- Updated phpunit to support versions 10.5 and 11.0
- Updated mockery to support version 1.6
Updated CI/CD configuration:
- Updated GitHub Actions checkout action from v2 to v4
- Added fail-fast: false to test matrix
- Added appropriate version exclusions for Laravel 11/12 with older PHP versions

Implementation Details

The custom renderHtmlAttributes() method:

Iterates through the attributes array
Handles numeric keys by using the value as the key
Escapes attribute values using Laravel's e() helper
Returns a properly formatted HTML attribute string with leading space, or empty string if no attributes

- Update illuminate/* constraints to include ^11.0|^12.0 - Remove abandoned laravelcollective/html dependency, replace HTML::attributes() with inline renderHtmlAttributes() method - Update dev dependencies: orchestra/testbench ^9.0|^10.0, phpunit/phpunit ^10.5|^11.0, mockery/mockery ^1.6 - Update phpunit.xml for PHPUnit 10+ compatibility (remove deprecated attributes, use <source> instead of <filter>) - Update CI matrix: add PHP 8.3/8.4, Laravel 11/12, proper version exclusions, actions/checkout@v4, fail-fast: false https://claude.ai/code/session_01Bh7hJQuxycYMCdyWhLabpr

Copilot

Pull request overview

This PR modernizes the laravel-menu package by removing the dependency on laravelcollective/html and implementing custom HTML attribute rendering, while extending support to Laravel 11 and 12 with PHP 8.3 and 8.4.

Changes:

Removed laravelcollective/html dependency and replaced HTML::attributes() with a custom renderHtmlAttributes() method
Extended support to Laravel 11 and 12, and PHP 8.3 and 8.4
Updated test infrastructure for PHPUnit 10/11 compatibility and GitHub Actions

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/MenuItem.php	Added custom `renderHtmlAttributes()` method to replace `HTML::attributes()` functionality, removed HTML facade import
src/Provider.php	Removed HTML service provider registration and alias loader for laravelcollective/html
tests/TestCase.php	Removed HTML service provider from test setup
composer.json	Removed laravelcollective/html dependency, added Laravel 11/12 support, updated test dependencies for PHPUnit 10/11
.github/workflows/tests.yml	Added PHP 8.3/8.4 and Laravel 11/12 to test matrix with appropriate exclusions, updated checkout action to v4
phpunit.xml	Modernized configuration for PHPUnit 10/11 compatibility (updated filter/whitelist syntax, removed deprecated attributes)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T11:13:44Z

composer.json

@@ -19,16 +19,15 @@
   ],
   "require": {
        "php": "^8.0",


The test matrix excludes PHP 8.0 and 8.1 for Laravel 11, but Laravel 11 actually requires PHP 8.2 as the minimum version. The composer.json PHP requirement of "^8.0" is too permissive when Laravel 11 and 12 are included. Consider either:

Updating the minimum PHP version to 8.2 in composer.json to match Laravel 11's requirements, or

Adding exclusions for Laravel 11 with PHP 8.2 is not necessary (it's supported), but the composer.json should reflect that PHP 8.2+ is required when using Laravel 11+.

Suggested change

"php": "^8.0",

"php": "^8.2",

Copilot · 2026-02-19T11:13:45Z

src/MenuItem.php

+            if (is_numeric($key)) {
+                $key = $value;
+            }
+


The renderHtmlAttributes method doesn't handle boolean attributes correctly. In HTML5, boolean attributes like "disabled", "required", "checked" should be rendered without a value (e.g., just "disabled" not 'disabled="1"'). When a value is boolean true, the attribute name should be rendered alone. When false or null, it should be omitted entirely. The current implementation will render 'disabled="1"' for ['disabled' => true], which while functional, is not the standard HTML5 practice. Consider checking if $value is a boolean and handling it accordingly.

Suggested change

if (is_bool($value)) {

if ($value) {

$html[] = $key;

}

continue;

}

Copilot · 2026-02-19T11:13:45Z

src/MenuItem.php

+            if (is_numeric($key)) {
+                $key = $value;
+            }


When a numeric key is encountered and its value is also numeric, this code will use the numeric value as the attribute name (e.g., ['0' => '123'] becomes key=123 where key is "123"). This could lead to invalid HTML attribute names. Consider validating that the resulting key is a valid attribute name, or at minimum ensuring numeric-only attribute names don't get created since HTML attribute names cannot be purely numeric.

Copilot · 2026-02-19T11:13:45Z

src/MenuItem.php

+            }
+
+            if (!is_null($value)) {
+                $html[] = $key . '="' . e($value) . '"';


The attribute key ($key) is not escaped before being concatenated into the HTML string. While attribute values are properly escaped using Laravel's e() helper, malicious attribute names could potentially be injected if user input controls the attribute keys. Consider also escaping the attribute key to prevent potential XSS vulnerabilities, especially since attributes come from user-configurable menu items.

Suggested change

$html[] = $key . '="' . e($value) . '"';

$html[] = e($key) . '="' . e($value) . '"';

Copilot · 2026-02-19T11:13:45Z

.github/workflows/tests.yml

        exclude:
          - laravel: 10.*
            php: 8.0
+          - laravel: 11.*
+            php: 8.0
+          - laravel: 11.*
+            php: 8.1
+          - laravel: 12.*
+            php: 8.0
+          - laravel: 12.*
+            php: 8.1


Laravel 9 officially supports PHP 8.0, 8.1, and 8.2, but not PHP 8.3 or 8.4. The test matrix should exclude Laravel 9 with PHP 8.3 and 8.4 to prevent testing unsupported combinations. While they may work in practice, testing officially unsupported version combinations can lead to false positives or obscure compatibility issues.

Copilot · 2026-02-19T11:13:45Z

src/MenuItem.php

    public function getAttributes()
    {
        $attributes = $this->attributes ? $this->attributes : [];

        Arr::forget($attributes, ['active', 'icon', 'search_keywords']);

-        return HTML::attributes($attributes);
+        return $this->renderHtmlAttributes($attributes);
+    }


The existing test coverage for getAttributes() only tests a simple string attribute case. Consider adding test cases for edge cases such as:

Boolean attribute values (e.g., ['disabled' => true])

Null values (to ensure they're properly omitted)

Special characters in attribute values (to ensure proper escaping)

Multiple attributes

Numeric keys with string values

Empty attribute arrays

This is especially important since the implementation has changed from using a well-tested library to a custom implementation.

denisdulici requested a review from Copilot February 19, 2026 11:07

Copilot started reviewing on behalf of denisdulici February 19, 2026 11:07 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove laravelcollective/html dependency and add Laravel 11/12 support#10

Remove laravelcollective/html dependency and add Laravel 11/12 support#10
johnnyhawley wants to merge 1 commit intoakaunting:masterfrom
johnnyhawley:claude/laravel-11-12-support-lbBjB

johnnyhawley commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

+            if (is_bool($value)) {
+                if ($value) {
+                    $html[] = $key;
+                }
+                continue;
+            }

	$html[] = $key . '="' . e($value) . '"';
	$html[] = e($key) . '="' . e($value) . '"';

Conversation

johnnyhawley commented Feb 13, 2026

Summary

Key Changes

Implementation Details

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants