Adds a span to contain the text on all crud buttons

[promatik]

@gabacode on gitter asked for an easy way to hide the text on table buttons. I needed this myself previously. For now I came up with the idea of shrinking the width of the button and "overflow hide" the text. It a very hacky way to solve this.

This PR makes it easier to access and hide the text (without icons) inside buttons. One may do now something like; #crudTable .btn span { display: none }.

[promatik]

I end up adding the span container to other crud buttons, it may be useful. If there is an easy way do it without this PR, please let me know 😂

[pxpm]

I don't think it can get easier than this.

The other solution would be to use some regex in element.innerHtml and keep only the icon part or move the icon away from the text, either by using pseudo css selectors like :before or some classes in the anchor.

You also have the option to override all the buttons, but it's a big effort for such a small change.

Down the road you can also add a crud setting hideTableButtonsText => true for the ListOperation.

Best,

[gabacode]

This is a great solution but it sems to be breaking when setupShowOperation(), on my side. Unless display: inline-flex is also added to the class, otherwise it will show as such: A crud setting for this option would be great!

[promatik]

@gabacode what solution is that screenshot of? The overflow hidden approach or the span?

[gabacode]

@gabacode what solution is that screenshot of? The overflow hidden approach or the span?

@promatik that's the span with overflow hidden, but from the "Preview" (/show) mode

[promatik]

@gabacode the idea of the <span> option, is to avoid the overflow: hidden plus width: forced px and use display: none directly to the span.

@pxpm where do you think we should add the setter and the getter for the crud config? I add it CrudPanel/Traits/Buttons.php not sure if it's the right choice. The crud config is a good idea 👌 this way you can add it directly on crud.configs, or you can ignore this and create your own css rules, perhaps with media queries.

[gabacode]

@gabacode the idea of the <span> option, is to avoid the overflow: hidden plus width: forced px and use display: none directly to the span.

Got it! I now added the spans, much better.

[pxpm]

@promatik I think instead of using getters and setters in the crud panel we should use the Settings Api so in setupListOperation() dev could:

// .... columns etc

CRUD::setOperationSetting('hideTableButtonsText', true); 

[tabacitu]

I agree with @pxpm - the Settings API should be better here. Since this is non-essential functionality, let's not bloat up the Buttons class with something just one operation uses.

---

Now, I do agree this is a good change. And I see where it's needed - I have some CRUDs with a lot of buttons too and it's not the best UX. So I want a solution too.

But I'm also worried that if we hide the button texts:

1. The buttons will become to small to easily click (especially on phone/tablet)
2. The buttons will not be intuitive enough, with the icon alone, especially for custom buttons; normally I just pick something that's closest to what I need; but except for Edit and Delete, you wouldn't know what a button does until you click it, if it didn't explicitly say next to it "Mark as Paid" or "Send reminder" or something like that; so default buttons will probably just fine; but custom buttons will probably need the text to be clear what they do;

What do you guys think about this? Valid worries right?

Ideas:

• For (1) we could make the icon a little bigger if the text is hidden; don't know how good it'd look though;
• For (2):
  • on desktop - we could add a tooltip; We should probably trigger the Bootstrap tooltip globally anyway, so that it's easy for people to add tooltips to their stuff by just adding data-toggle="tooltip" title="Mark as Paid". If that happens (globally triggered tooltips) we could just make sure buttons have tooltips - which would solve the problem on Desktop. But...
  • on mobile - the action column is probably hidden, so it makes no sense to hide the button texts here; I think it would make for a better UX to show the buttons with texts, because they are easier to read and easier to click;
  • on tablet - here's where it's trickier; because the action column could be hidden, but it could not; and you also have no tooltips like you do on desktop; so over here it's probably better to just assume the same as mobile and NOT hide the button texts;
  • which brings me to the conclusion that... maybe we should only hide the button texts if the action column is not hidden? is that the solution? I don't know, I'm asking - what do you guys think?

[promatik]

@tabacitu on Desktop I have already added a tooltip, and I agree, icons should be slightly bigger. And I also agree this works good on desktop, not so good on mobile. So by default, I think we should provide this solutions for desktop only.

Tell me what you think about the new changes:

Desktop

Mobile/Tablet

Tooltip:

[pxpm]

Hello guys.

I agree with both of you about visibility and UX. But I still want my tablet icons to don't show text, I am the developer. Maybe I have only one action, and don't need labels at all.

If we are pushing some change in this behavior we better be flexible and don't limit to our understanding of what could be better for user. Sure, I think we have good understanding of what works best as default for the user, but in the end we are building software for developers.

'showButtonLabels' => [
    'mobile' => true, //we can use mobile up to tablets, because like tabacitu said if it's small screen (not tablet) actions is almost certain hidden.
    'desktop' => true
];

I am pretty sure someday we will be talking about showing label in one action and not in another. It's just how things goes.

What you think @promatik ?

[pxpm]

I missed but I totally agree. Sorry for that. It's good enough that we have that solution. I think if it's documented for who wants to to change that it's easy enough to understand. I was just not thinking in that direction 🤣

Best, Pedro

[promatik]

@pxpm just refactored this, can you give it another try?

[tabacitu]

Please read my comment above - https://github.com/Laravel-Backpack/CRUD/pull/3320#discussion_r546680313

I'm moving this to 4.2 because one thing none of us remembered is that "This is a FEATURE". And we said we're in a feature freeze in 4.1 😄

[scrutinizer-notifier]

The inspection completed: No new issues

[tabacitu]

This has spiraled out of control, so I'm shutting it down. We thought this would be a quick change ("just add span to all buttons") but it's become a lot more, and I don't agree with it:

• I don't agree with making it too easy for people to just toggle "hide labels"; icons without labels is bad UX;
• I don't agree with making it different on mobile or tablet;
• we already have a solution to make the buttons smaller nowadays, which is compressing all the buttons into a dropdown, by doing 'lineButtonsAsDropdown' => true in your config/backpack/operations/list.php;

For the reasons above, I'm closing this PR. If you guys think I'm wrong, and we should add spans to buttons, then fine, create a PR that does that and only that. Should be simple to do and merge. But more than that? No.

Let's be more mindful, and not let ideas spiral like this any more, please.

Cheers!