Laravel-Backpack / CRUD

Build custom admin panels. Fast!
https://backpackforlaravel.com
MIT License
3.16k stars 893 forks source link

Bug on table responsive mode #1489

Closed kevind11 closed 6 years ago

kevind11 commented 6 years ago

Bug report

What I did:

I update my laravel-backpack to latest version

What I expected to happen:

The table is responvsive before

What happened:

Now the table is not responsive and no button for display the modal information or each row

What I've already tried to fix it:

Already fix it by removing this line of code from datatables_logic.blade.php

initComplete: function () {
            crud.responsiveToggle(
                jQuery('#crudTable').DataTable()
             );
        }

Backpack, Laravel, PHP, DB version:

My backpack version is 3.4.17, laravel 5.6, PHP ##7.1.13

lloy0076 commented 6 years ago

What do you mean by "not responsive":

kevind11 commented 6 years ago

the table didnt behave differently on different screen size

swilla commented 6 years ago

I'm having a similar issue. After upgrading to the latest, I no longer get the three dots on the left side of each row. The action buttons are hidden outside the viewport. It doesn't appear that it actually in responsive mode anymore.

SuperDooker commented 6 years ago

The problem is urgent, I have the same thing. On mobile screens, the table does not behave as stated

tomgillespy commented 6 years ago

+1 for this issue - the three dots menu on the left isn't appearing in 3.4.17. I've downgraded to 3.4.16 and its fine once you execute artisan view:clear to clear the compiled views.

OwenMelbz commented 6 years ago

I think there's a bit of trouble here.

So there are a couple of problems.

Scenario 1

if you have 10 columns, 5 are on screen, 5 are off screen

You turn off the visibility of the first 5 - expecting to see the next 5.

They never appear.

If you trigger the responsive rebuild and recalculations, then they appear.

This fix enables the calculations by default to fix the issue before it happens

The Fix

The code you've provided is what fixes the above bug in Datatables.

Scenario 2

You want it to collapse like it used to

The Fix

In short... You cant really, the previous effect seems to be helped by the bug, HOWEVER, that's not to say there isn't an alternative solution.

Most frameworks offer features such as "responsive-table" classes such as Bootstrap, Foundation, UIKit etc.

This is no different - however, to get it to work you need to modify the datatables_logic.blade.php and make the following change

-      "<'row'<'col-sm-12'tr>>" +
+      "<'row table-wrapper'<'table-inner'tr>>" +

This will give it the functionality of things like bootstrap and allow "responsive tables" - however this is NOT collapsable tables which it previously did.

Maybe its a bigger discussion to see if more changes can be made to fix.

OwenMelbz commented 6 years ago

@tabacitu if you can replicate then this might be a suitable fix https://github.com/Laravel-Backpack/CRUD/pull/1497 by making the table scrollable

tabacitu commented 6 years ago

Thanks a lot for reporting this @kevind11 , @swilla , @SuperDooker @tomgillespy . I've just pushed a hot fix.

composer update should fix it for you.

Sorry for pushing this into production - looks like this change was the culprit. I removed it for now and we'll find a different way of achieving the same thing.

OwenMelbz commented 6 years ago

@tabacitu but noooooooo you've now broken the column visibility lol, By removing that code there is a bug. By adding the new code you remove a bug but make a change to the UI

tabacitu commented 6 years ago

@OwenMelbz haha I know, uncomfortable solution for now. But I think it was the best way to go for a hotfix.

The way I see it:

Hence, the hotfix, until we find a solution for both. Can't change the UI that much (remove a functionality entirely) without calling it a breaking change. Otherwise, you know... pitchforks :-)

tabacitu commented 6 years ago

Just merged #1499 that properly fixes this. I'll be available with a composer update in a few hours/day, as 3.4.19

Cheers!

ghost commented 6 years ago

Is there a way to have a horizontal scroll bar instead of the 3 dots ?

OwenMelbz commented 6 years ago

šŸ˜‚šŸ˜‚šŸ˜‚šŸ˜‚ Give @tabacitu a couple of years and he'll realise the terrible mistake :P #antiPatternLife

tabacitu commented 6 years ago

:-)) @OwenMelbz thatā€™s been know to happen, I know - you in particular have had to wait years for me to turn around, thank you for understanding :-)

In this particular case, though, I havenā€™t changed my mind yet - I still think the three dots offer a better UX on mobile/tablet, than the scrollbar. So itā€™s still the default. But we do provide a way to toggle it off. Two ways, actually:

This will make the horizontal scrollbar appear - what Bootstrap and others call ā€œresponsiveā€, but I personally wouldnā€™t :-)

Iā€™ve just realised this feature isnā€™t properly documented. @umbrellait-sergey-krivosheenko please check out the new docs for it here: https://backpackforlaravel.com/docs/3.4/crud-operation-list-entries#responsive-table - thank you for pointing this out.

Cheers!

ghost commented 6 years ago

Awesome! Thanks a lot!)

On Fri, Oct 26, 2018 at 10:37 AM Cristian Tabacitu notifications@github.com wrote:

:-)) @OwenMelbz https://github.com/OwenMelbz thatā€™s been know to happen, I know - you in particular have had to wait years for me to turn around, thank you for understanding :-)

In this particular case, though, I havenā€™t changed my mind yet - I still think the three dots offer a better UX on mobile/tablet, than the scrollbar. So itā€™s still the default. But we do provide a way to toggle it off. Two ways, actually:

  • $this->crud->disableResponsiveTable(); for one CRUD panel;
  • 'responsive_table' => false, in the config/backpack/crud.php file for all CRUD panels;

This will make the horizontal scrollbar appear - what Bootstrap and others call ā€œresponsiveā€, but I personally wouldnā€™t :-)

Iā€™ve just realised this feature isnā€™t properly documented. @umbrellait-sergey-krivosheenko https://github.com/umbrellait-sergey-krivosheenko please check out the new docs for it here: https://backpackforlaravel.com/docs/3.4/crud-operation-list-entries#responsive-table

  • thank you for pointing this out.

Cheers!

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Laravel-Backpack/CRUD/issues/1489#issuecomment-433316431, or mute the thread https://github.com/notifications/unsubscribe-auth/ApPHYDCz8-ImjkqOuMjrrhMY4jAD14Ckks5uoruygaJpZM4U-WLP .

-- SERGEYKRIVOSHEENKO WEB DEVELOPER Horse rider Tel: +7-952-569-69-87

Umbrella IT https://umbrellait.com/

Your successful web and mobile development partner.

FACEBOOK https://www.facebook.com/umbrellaitcom/ *| INSTAGRAM https://www.instagram.com/umbrellaitcom/ | ABOUT US* https://umbrellait.com/

jrbecart commented 4 years ago

If you migrate from BP3 to BP4, move $this->crud->disableResponsiveTable(); from setup() to setupListOperation()