Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refactor default theme #60

Open
ronilaukkarinen opened this issue Feb 19, 2021 · 2 comments
Open

Refactor default theme #60

ronilaukkarinen opened this issue Feb 19, 2021 · 2 comments

Comments

@ronilaukkarinen
Copy link

First of all, good work! This navigation is beautiful and really works out of the box with WordPress and our ultra minimal and modern air-light theme.

I'd like very much to contribute with my first issue. I noticed theme default base is really hard to understand because it contains

  • Too much nesting
  • Too specific selectors
  • Too much overrides with specificity, :not and !important that should be avoided
  • Unnecessary & and > in wrong places, they should be after { not before
  • Duplicate selectors

Stylelint with our pretty reasonable standards causes 631 errors. Had to put in stylelint-disable sh-waqar/declaration-use-variable, no-descending-specificity, selector-max-combinators, selector-max-pseudo-class, selector-max-class, selector-max-compound-selectors, selector-max-specificity, declaration-no-important, max-nesting-depth, no-duplicate-selectors for now.

For example selectors of this next secenario should be in one line because in-between there's no specifications, it's also way too specific:

carbon (7)

So my suggestion is to simplify default theme structure by refactoring it completely. This would ensure people should have easy time creating their own themes. I personally had to do overrides because this overkill at the moment.

I used to nest too much in my past as well so I'm not blaiming anyone, we've all been there. I can gladly help in some point in the future, but right now I don't have enough time for anything else to bring this to your attention.

@somewebmedia
Copy link
Owner

The problem is that the menu has many features and subnav possibilities and options, which are targeted by these very specific complex selectors. I understand you, at one time I've completely lost it and didn't know what is what anymore (I still don't) :D

The problem with these out of the box solutions that (try to) fit all situations is that there are just too many variations that could happen that we have to cover. I tried refactoring it once and this is what I've ended up with. I had to have opened like 10 tabs for all different situations to test against, it wasn't fun at all :D

I'm open for suggestions about the workflow.

@ronilaukkarinen
Copy link
Author

Haha I understand your pain, I've been there myself as well. I may have some thoughts to this later...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants