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

Use Spatie/html instead of laravelcollective/html #21

Closed
vpratfr opened this issue Dec 22, 2017 · 3 comments
Closed

Use Spatie/html instead of laravelcollective/html #21

vpratfr opened this issue Dec 22, 2017 · 3 comments

Comments

@vpratfr
Copy link

vpratfr commented Dec 22, 2017

This would help make the code of presenters much cleaner. For instance, an existing method:

    return '<li' . $this->getActiveState($item) . '>
		<a href="' . $item->getUrl() . '" ' . $item->getAttributes() . ' class="nav-link">'
        . $item->title . '</a></li>' . PHP_EOL;

Note that above code is using some rendering functionnality placed in the MenuItem class (which should not be there, see #19)

Using Spatie/html, we can rewrite this as

    $link = html()->a($item->getUrl())
        ->addClass('nav-link')
        ->attributes($item->attributes)
        ->html($item->title);

    return html()->element('li')
        ->addClass('nav-item')
        ->addClassIf($item->isActive(), 'active')
        ->addChild($link);
@nWidart
Copy link
Owner

nWidart commented Dec 22, 2017

Hello,

I'm not really a fan of using a this.
Since it's in presenters, you should already be able to use it in your own presenters.

@vpratfr
Copy link
Author

vpratfr commented Dec 22, 2017

This could also simplify the way HTML is rendered. Presenters would return Elements instead of returning strings. Which allows manipulating the elements (add class, add children, change attributes, etc.).

@vpratfr
Copy link
Author

vpratfr commented Mar 17, 2018

Nevermind. Ended up rewriting your package for a few things I needed.

Fixed in https://github.com/marvinlabs/laravel-menus/

@vpratfr vpratfr closed this as completed Mar 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants