search

gobrightspot / nova-detached-actions

A Laravel Nova tool to allow for placing actions in the Nova toolbar detached from the checkbox selection mechanism.
MIT License
168 stars 50 forks source link

Poll: Would you prefer that the DetachedAction button style match the Nova primary button? #8

Closed gobrightspot closed 4 years ago

gobrightspot commented 4 years ago

We had an early contribution from @milewski posing the question/asserting his opinion about whether we should match the style of the default primary button style in Nova or whether we should use a 'secondary' color based on the navigation menu background color as we've done in the first iteration of the package.

Option 1: Switch to Nova primary button and provide css classes allow developer to create styles in a custom theme

We could switch to btn-primary but keep the package css class, .btn-detached-action. Instead of actually providing css styles for those classes, we would remove them and allow developers to style using the existing custom theme documentation. @milewski also argued that it wasn't worth the extra HTTP request for the styles. He may have a point.

Option 2: Keep the button style as is and still allow the developer to theme

Since we already use .btn-detached-action, developers could still easily provide styles via a custom theme.

Vote

If you care about either option, please leave a vote below, just write Option 1 or Option 2 if you would like to leave your opinion too, that's definitely appreciated.

If you don't want to write a comment, feel free to just add a reaction: ❤️for Option 1 👍for Option 2

I'll leave the issue open for a while to see if we can get some votes in.

Thanks!

milewski commented 4 years ago

Hey among that changes mentioned above I have also made 3 other improvements on my fork that you could incorporate in here:

1 - https://github.com/dcasia/nova-detached-actions/blob/master/src/DetachedAction.php#L13 implements PotentiallyMissing interface to remove itself from the main action filter menu from nova (current behavior shows the action button and it is still available within the action select input in nova.. this fix that)

2 - https://github.com/dcasia/nova-detached-actions/blob/master/resources/js/components/CustomIndexToolbar.vue#L41 simplify the js code by reusing most of the functionality already written on the nova HandlesActions mixin.. and overriding only what needs to be changed

3 - Fix a bug where the handle method wouldn't be called

gobrightspot commented 4 years ago

@milewski Awesome suggestions, thanks!

Are you interested in submitting a pull request?

milewski commented 4 years ago

Well, my fork incorporates all the changes you are waiting for votes on this thread... if that`s what people want I can submit that as a PR otherwise I have to re-do the changes again from this repo master branch and submit these changes individually...

gobrightspot commented 4 years ago

Your call, if you want to wait to see if people bother to vote on this, I understand.

I have some changes in the works too that I think will be pretty useful, I’d love to get yours and mine together for the next release but I get it if you don’t want to make atomic commits for each one.

I can always make the changes myself too if you don’t have time.

Thanks again for contributing.

vesper8 commented 4 years ago

My preference is to have the detached action buttons look different than the create button, but more importantly, I'd like it if we could customize the class by passing one or more classes via withMeta

MikeCraig418 commented 4 years ago

My preference is to have the detached action buttons look different than the create button, but more importantly, I'd like it if we could customize the class by passing one or more classes via withMeta

This right here! Also, I'd like to add an image or an icon.

eoghanobrien commented 4 years ago

My preference is to have the detached action buttons look different than the create button, but more importantly, I'd like it if we could customize the class by passing one or more classes via withMeta

This right here! Also, I'd like to add an image or an icon.

I agree an icon would be pretty awesome. It's not so simple to implement though, once you afford the ability to add an icon, the expectations change a little. Which icon set? The "official" is the heroicons set. Are they sufficient? Do you expect to be able to use any icon you want? What format? SVG or an Image or both? How do you pass the icon into the component from the Nova action in PHP? Do you really want to pass in big HTML string?

I have a branch to add support for classes. Any interest In trying it out?

babul commented 4 years ago

I vote for Option 1, as I would like detached actions to follow Nova, but be able to specify classes to customize that behavior.

Any support to make it easier to style buttons would be most appreciated!

Icon support would be fantastic, but like @eoghanobrien mentioned... how best to implement? SVG is most useful IMHO as it lets anyone use any icon they can get in SVG format, which is pretty easy nowadays.

eoghanobrien commented 4 years ago

Option 1 has been added in v1.1.0 - see the README.md for more

eoghanobrien commented 4 years ago

@949mac @babul v1.1.1 added icon support and some other useful updates, check the README.md