[Bug] Fix issues found in CoreUI PR #24

Laravel-Backpack / theme-coreuiv4

UI for Backpack v6 that uses CoreUI v4 and Bootstrap v5.

MIT License

7 stars 4 forks source link

[Bug] Fix issues found in CoreUI PR #24 #26

Closed tabacitu closed 1 year ago

tabacitu commented 1 year ago

Damn it!I merged https://github.com/Laravel-Backpack/theme-coreuiv4/pull/24 by mistake... when I merged another PR.

The issues there still stand, the ones in my inline comments - so please let's fix them in a different PR:

karandatwani92 commented 1 year ago

@tabacitu I need your perspective here.

Your question is, Why are we adding this?

+ html:not([dir=rtl]) .wrapper {
+     padding-left: var(--cui-sidebar-occupy-start, 0)
+ }

+ *[dir=rtl] .wrapper {
+     padding-right: var(--cui-sidebar-occupy-start, 0)
+ }

Previously we were using CSS from the current official demo. https://coreui.io/demos/bootstrap/4.2/free/css/style.css and now we replaced it with CDN which is officially documented. https://cdn.jsdelivr.net/npm/@coreui/coreui@4.2/dist/css/coreui.min.css

When I compared these two, the demo CSS has the above padding code. Core-UI people are using it in their demo, so I added it to our project.

This worked, now what do think about it?

tabacitu commented 1 year ago

Hmm... how do the pages look different, with & without the code?

The fact that it's in the demo... but not in the official release... makes me think that it isn't mandatory. That it's done to prettify the demo alone, for some reason.

karandatwani92 commented 1 year ago

@tabacitu without code it becomes: