Laravel-Backpack / theme-coreuiv4

UI for Backpack v6 that uses CoreUI v4 and Bootstrap v5.
MIT License
7 stars 4 forks source link

[v6][Bug] CoreUIv4 always have two scrollbars #15

Closed tabacitu closed 1 year ago

tabacitu commented 1 year ago

Reported by Pedro with image:

I think he's referring to the horizontal scrollbar, at the bottom. That looks like it's there even when not needed (it should almost never be needed).

karandatwani92 commented 1 year ago

I tested it on Mac and Windows browsers:

coreui-v4 theme needs more attention than we expected.

phpfour commented 1 year ago

Hey @karandatwani92 @tabacitu

I was able to get the sidebar to scroll if I use the latest version of Simplebar styles in resources/views/inc/theme_styles.blade.php (here's the PR: https://github.com/Laravel-Backpack/theme-coreuiv4/pull/23):

++ @basset('https://unpkg.com/simplebar@latest/dist/simplebar.css')
-- @basset('https://coreui.io/demos/bootstrap/4.2/free/vendors/simplebar/css/simplebar.css')
-- @basset('https://coreui.io/demos/bootstrap/4.2/free/css/vendors/simplebar.css')
@basset('https://coreui.io/demos/bootstrap/4.2/free/css/style.css')

The horizontal scrollbar doesn't appear on my Mac but there is a high probability that it would also get resolved through this change - can you please test that?

@tabacitu I am a bit curious why we need to load this file from coreui's demo. If we need some of its stylings, then we should probably bring those in our own custom CSS file.

@basset('https://coreui.io/demos/bootstrap/4.2/free/css/style.css')

Thanks!

tabacitu commented 1 year ago

@tabacitu I am a bit curious why we need to load this file from coreui's demo. If we need some of its stylings, then we should probably bring those in our own custom CSS file.

We don't need to do that. In fact, it's a little dangerous to do that. Whenever possible, we should load the files from respectable CDNs - like you did from unpkg.com. Otherwise we risk CoreUI moving the demo to a different URL and BOOM, this theme no longer works.

karandatwani92 commented 1 year ago

@phpfour @tabacitu

I'm trying a different approach.

I don't get why are we using many CSS files from the demo while official documents instruct us to use only one.

+ @basset(https://cdn.jsdelivr.net/npm/@coreui/coreui@4.2.0/dist/css/coreui.min.css)
- @basset('https://unpkg.com/bootstrap@5.2.3/dist/css/bootstrap.min.css')
- @basset('https://unpkg.com/@coreui/coreui@4.2.4/dist/css/coreui.min.css')
- @basset('https://unpkg.com/animate.css@4.1.1/animate.compat.css')
- @basset('https://unpkg.com/noty@3.2.0-beta-deprecated/lib/noty.css')

- @basset('https://coreui.io/demos/bootstrap/4.2/free/vendors/simplebar/css/simplebar.css')
- @basset('https://coreui.io/demos/bootstrap/4.2/free/css/vendors/simplebar.css')
- @basset('https://coreui.io/demos/bootstrap/4.2/free/css/style.css')

This fixes the scroll bar without a third CSS. Although I believe this will solve other issues too. I'm giving it a try, will come up with a conclusion in a while. Thanks

phpfour commented 1 year ago

@karandatwani92 Awesome mate, that should be the right path.

karandatwani92 commented 1 year ago

PR submitted here https://github.com/Laravel-Backpack/theme-coreuiv4/pull/24