html-object icon indicating copy to clipboard operation
html-object copied to clipboard

Fix removing class

Open tortuetorche opened this issue 2 years ago • 3 comments

Hi @Anahkiasen,

This pull request fix a bug when using the HtmlObject\Traits\Tag::removeClass($classes) method.

Otherwise, when you try to remove a CSS class which isn't present, it'll remove a wrong CSS class, see the testCanRemoveClasses test for more info.

Because the array_search() PHP function returns the key for needle if it is found in the array, false otherwise. This array_search() function may return Boolean false, but may also return a non-Boolean value which evaluates to false.

So in the HtmlObject\Traits\Tag::removeClass() method of this package, we need to check if the $exists value is strictly not equals to false instead of using the !is_null($exists) check.

If this fix is merged and a new release is tagged, I'll update the Former dependency to release the final version of Former 5.0.0 🤞

By the way, many thanks for all the work you have done on this package and Former 👍

Have a good day, Tortue Torche

tortuetorche avatar Nov 17 '21 13:11 tortuetorche

My 2 cents, this would be way faster:

    public function removeClass($classes)
    {
        if (!is_array($classes)) {
            $classes = explode(' ', $classes);
        }

        $thisClasses = explode(' ', Helpers::arrayGet($this->attributes, 'class'));
        $this->attributes['class'] = implode(' ', array_diff($thisClasses, $classes));

        return $this;
    }

kylekatarnls avatar Nov 17 '21 22:11 kylekatarnls

Hi @kylekatarnls,

Thank you for your suggestion, according to my tests it works 👍 @Anahkiasen Should I update this pull request with the @kylekatarnls code?

There is also another bug in the HtmlObject\Traits\Tag file, the is_empty() function isn't a valid PHP function, we should replace it by empty(). Should I add this fix to this pull request or create a new one?

Have a good day, Tortue Torche

tortuetorche avatar Nov 19 '21 08:11 tortuetorche

It sounds like this repo is possibly no longer maintained so I forked it. https://github.com/kylekatarnls/html-object

I switched to GitHub Actions up to PHP 8.3 and merged compatibility fixes and your fix.

I would be happy to review/merge other fixes and grant access to the repo to those who might want to contribute on a regular basis.

It can be installed with:

composer remove anahkiasen/html-object
composer require kylekatarnls/html-object

kylekatarnls avatar Feb 26 '23 11:02 kylekatarnls