Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove some old CSS and fix selector #3352

Merged
merged 2 commits into from
Dec 2, 2020
Merged

Remove some old CSS and fix selector #3352

merged 2 commits into from
Dec 2, 2020

Conversation

martijnb92
Copy link
Contributor

This PR removes some old CSS which does nothing, .tab-content has a .p-0 class for example.
It also fixes the .nav-tabs selector to only select elements in the custom nav-tabs bar, otherwise it also selects menu items in top_right_content blade file.

Maybe it's an idea to move the remaining CSS and the list.css contents to the Backstap package (https://github.com/DigitallyHappy/BackStrap/blob/master/src/scss/_custom.scss)
And use this files strictly for user overrides.

@promatik
Copy link
Contributor

Thank you for this @martijnb92, good catch.

These files need a clean up for sure, and yes, the idea is to move the code somewhere developer don't need to see it, and leave these files (like form.css) for developer custom rules.

Copy link
Contributor

@promatik promatik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may look like a breaking change, some developers may be relying on those rules now (very unlikely though) but these developers are not going to republish assets, so it won't break anything, and the rules are redundant, there are other rules overriding it.

TLDR; I think it's safe to merge this now.

@tabacitu
Copy link
Member

tabacitu commented Dec 1, 2020

Thanks guys! While testing this I stumbled unto a small bug kind of annoying bug. The border-bottom is still showing on :hover, even if the tab is already selected:
2020-12-01 09 53 05

At first I thought it was caused by this PR, because I've never seen this before in Backpack. But... apparently it also shows up using the OLD css, but... in fewer cases. Which again is surprising - it's the first time I see this. Couldn't tell you what those cases are though, sometimes it works sometimes it doesn't... 😡 It looks like maybe it happens when you hover when the page isn't completely loaded... maybe?! But then sometimes it seems to happen when the page is fully loaded too... Which is... yeah... 😔

@promatik could you please take a look at this too? Since we're doing changes to this file, it's best to have all annoyances fixed in one change, so that when people publish the assets they get the best.

@martijnb92
Copy link
Contributor Author

Good catch, that's caused by some other borders when the bottom border is hidden.
Will update this PR.

@tabacitu tabacitu requested a review from promatik December 1, 2020 15:28
@scrutinizer-notifier
Copy link

The inspection completed: No new issues

Copy link
Contributor

@promatik promatik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martijnb92 great job!
Indeed the problem with the border-bottom was happening, and now it's not! 👌
Thank you

@tabacitu
Copy link
Member

tabacitu commented Dec 2, 2020

Awesome. Thanks guys, merging!

@tabacitu tabacitu merged commit 5056b6c into Laravel-Backpack:master Dec 2, 2020
@martijnb92 martijnb92 deleted the old-styles branch December 3, 2020 09:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants