Jarzka / stylefy

Clojure(Script) library for styling user interface components with ease.
MIT License
317 stars 10 forks source link

Overriding bootstrap styles #17

Open jiangts opened 6 years ago

jiangts commented 6 years ago

Perhaps I missed something, but simply calling use-style on components from React-Bootstrap ends up with the .stylefy-ID class being lower precedence than the bootstrap.min.css styles. Any idea why, or am I missing something simple?

Jarzka commented 6 years ago

In most cases, stylefy's style definitions should have stronger CSS selectors than Boostrap. I have used Boostrap CSS in the examples project and all stylefy's style definitions override Boostrap style definitions.

Make sure stylefy's style tags are defined after including Boostrap CSS in your HTML. This is important so that stylefy's style definitions get stronger selectors than Boostrap (the order does matter in CSS, if two selectors are the same precise, the one which is defined afterwards wins). If this does not help, can I take a look at your source code?

You can also use CSS !important rule in your style definitions to make sure that they have the strongest selectors and should always override everything else. However, in most cases you should not need to do this. The usage of !important should be an exception rather than a rule.

fwaddle commented 6 years ago

I've just run into the same problem today. The !important saved me but not sure what's happening here.

So my example is a simple anchor without a href:

(def my-button-style {:border-width          :2px
                      :border-radius         :8px
                      :width                 :200px
                      :border-color          "black"
                      :color                 "black"
                      ::stylefy/with-classes ["btn-block btn p-3 m-4"]
                      ::stylefy/mode         {:hover {:background-color "black"
                                                      :color            "white"}}})
(def my-button-title-style {:color  "inherit"})

[:a (use-style my-button-style {:on-click #(println "Hello there")})
  [:h4 (use-style my-button-title-style) "My Button"]]

I expect the button title to go white when I hover over it as the background goes black. It doesn't because BS has a a:not([href]):hover which sets the 'color' to 'inherit'.

Anyway if I add the "!important" option like below, then it works:

(def my-button-style {:border-width          :2px
                      :border-radius         :8px
                      :width                 :200px
                      :border-color          "black"
                      :color                 "black !important"
                      ::stylefy/with-classes ["btn-block btn p-3 m-4"]
                      ::stylefy/mode         {:hover {:background-color "black"
                                                      :color            "white !important"}}})

Also here is my html file's <head>:

  <head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
    <link href="css/style.css" rel="stylesheet" type="text/css">
    <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous">
    <link rel="icon" href="https://clojurescript.org/images/cljs-logo-icon-32.png">

    <script src="https://code.jquery.com/jquery-3.2.1.slim.min.js" integrity="sha384-KJ3o2DKtIkvYIK3UENzmM7KCkRr/rE9/Qpg6aAZGJwFDMVNA/GpGFF93hXpG5KkN" crossorigin="anonymous"></script>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.12.9/umd/popper.min.js" integrity="sha384-ApNbgh9B+Y1QKtv3Rn7W3mgPxhU9K/ScQsAP7hUibX39j7fakFPskvXusvfa0b4Q" crossorigin="anonymous"></script>
    <script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/js/bootstrap.min.js" integrity="sha384-JZR6Spejh4U02d8jOt6vLEHfe/JQGiRRSQQxSfFWpi1MquVdAyjUar5+76PVCmYl" crossorigin="anonymous"></script>

    <style id="_stylefy-constant-styles_"></style>
    <style id="_stylefy-styles_"></style>
  </head>
Jarzka commented 6 years ago

Some of the style definitions in Boostrap truly have stronger (more precise) selectors than stylefy's autogenerated class names and thus are applied, even if stylefy's style tags are defined afterwards in HTML. This becomes down to the fact how CSS applies styles when multiple style definitions are targeting the same element. The order does matter, but more precise selector is usually going to win.

For example, Boostrap's definition a:not([href]):hover is more precise than a simple autogenerated class name .stylefy-ID

I have noticed this problem a few times but don't yet know how we should fix it. I see a couple of ways to do this:

The last two options would be possible to achieve via stylefy/init options map, something like :use-stroger-selectors? or something. This would not break anything, it would just be an option to make stylefy's selectors stronger in cases when it's important. Still, I'm not completely sure if this is a good idea or not.

fwaddle commented 6 years ago

I have no idea which is the 'right' way to do it, but happy to have the '!important' option to get things working for now.

Jarzka commented 6 years ago

There's probably not 'right' way to do this, I'm just thinking of a way which would feel right or at least not too hacky. :D If I think about writing CSS manually by hand, how would I fix this problem? Using the same selector as Boostrap would do the trick, but since stylefy only generates class selectors, this is not possible to achieve.

The only way to fix this is to make sure stylefy's style definitions are stronger than Boostrap's style definitions. Using !important is the current way to fix this, but I think I should at least test the idea of wrapping everything inside a media query which is always evaluated to true.

nijk commented 6 years ago

I'm just catching up on this as I've noticed some issues with CSS specificity although in a slightly different context.

I can see why there is a consideration for using the !important declaration as it is a sledgehammer for specificity issues. It's not something that is generally advisable as it leads the css down the slippery slope of always having to use it.

@fwaddle Would you be able to provide the following things so that I can help diagnose this issue:

  1. The output of Stylefy for the css selector (search the html for the unique classname) that uses ::stylefy/with-classes and is being overridden
  2. A screen shot of the CSS overriding the Stylefy style taken from the dev tools of your favourite browser
nijk commented 6 years ago

Actually I may have worked it out; AFAICT from the docs ::stylefy/with-classes should be able to solve "simple" specificity use cases like the one that @jiangts cites in the original post.

@fwaddle I've noticed that your implementation doesn't look right compared with the documentation:

Documented usage: ::stylefy/with-classes ["nav" "nav-pills"]

Implementation: ::stylefy/with-classes ["btn-block btn p-3 m-4"]

I think the above implementation will result in an invalid css selector, something like this: .btn-block btn p-3 m-4 instead of .btn-block .btn .p-3 .m-4. Therefore I think the fix for your implementation is:

::stylefy/with-classes ["btn-block" "btn" "p-3" "m-4"]

Jarzka commented 6 years ago

I came up with an idea how to resolve this issue (override Bootstrap styles easily):

What if we mount our application into div element with an id: <div id="my-app">

Then we ask stylefy to scope all style definitions inside that div, for example like this: (stylefy/init {:scope "#my-app"})

When we do this, all style definitions will be scoped inside this div. For example, when you use some style with stylefy, the CSS output will be something like this: #my-app ._stylefy_2324535 { color: 'red';}

When all styles are scoped inside #my-app, the style selectors should be stronger than what are defined in Boostrap.

nijk commented 6 years ago

@Jarzka Are you sure that this is necessary given my comment above? I believe that the ::stylefy/with-classes approach should work if it is used correctly. I haven't tested it but if one can supply the selector chain for the CSS then that will solve the specificity issues described.

Jarzka commented 6 years ago

I'm not sure how with-classes could solve this problem. All what with-classes does is that it simply attaches another classes into an HTML element along with stylefy's autogenerated class name. It does not solve the style conflict between Bootstrap style definitions and stylefy style definitions.

For example, if there is the following selector in Bootstrap: ul > li > a { color: "red" } Then the selector is going to be much stronger than a simple stylefy autogenerated class selector ._stylefy_343434 { color: "blue"} and the element is going to be red, instead of blue.

Thus, I think this problem could be solved by scoping stylefy's autogenerated classes inside the #my-app.

This would have stronger selector: #my-app ._stylefy_343434 { color: "blue"} than this: ul > li > a { color: "red" }

nijk commented 6 years ago

Sorry, my bad - I thought it added those classes to the selector chain in the CSS.

So going back to your idea, I'm assuming that the scope (stylefy/init {:scope "#my-app"}) would be optional? It will cause a CSS perf hit and should probably be a last resort. For that reason I think it would be better if the CSS selector could be suffixed with arbitrary classes/IDs on a per (use-style) basis, e.g.

(def my-button-style {
  :color "blue"
  ::stylefy/prefix-selector ["#foo" ".bar" "button"]}})

Would produce:

#foo .bar button ._stylefy_343434 { color: "blue"}

If you were to go down this route, a full implementation would probably include support for all CSS combinators

Jarzka commented 6 years ago

True. Bootstrap conflicts are still so rare that scoping every single autogenerated class would be an unnecessary perf hit (even though I believe that modern machines should be able to handle it quite easily). Your idea of doing manual scoping only when needed seems like a better idea.

Still, I'am a bit of sceptical of defining custom selectors with stylefy since one important aspect of stylefy is to get rid of writing CSS selectors manually. But maybe there is no another way?

nijk commented 6 years ago

I can't think of a better way if I'm honest

Jarzka commented 6 years ago

Neither can I. Perhaps writing selectors manually in those rare cases is the only reasonable way to go and thus stylefy should support it.

nijk commented 6 years ago

I think so, it's a real edge case

abustamam commented 6 years ago

One valid use case for allowing custom selectors is when you want to overwrite lower level items in a component, that aren't necessarily exposed. For example, in a React app I used, I was able to use Glamorous to overwrite some styles like so:

const DateRangePickerWrapper = glam.div({
  '& .DateRangePicker': {
    marginRight: 10,
    '& .CalendarDay': {
      border: 'none',
    },
    '& .CalendarDay__selected': {
      backgroundColor: 'blue',
    },
    '& .CalendarDay__selected_span': {
      backgroundColor: 'teal',
    },
  },
})

Used like so:

<DateRangePickerWrapper>
  <DateRangePicker {...props} />
</DateRangePickerWrapper>

I do stuff like this for almost every "css" library, including React Semantic UI. I haven't used Bootstrap with React.

I'd like to be able to do something like this using Stylefy. From what I can tell, this proposed syntax above might work?

Jarzka commented 6 years ago

I'm not familiar with Glamorous. What I see is that this generates CSS selector for the wrapper element and elements inside of it, right? I believe this is the same as attaching styles to child elements in HTML tags with stylefy (with sub-styles or not).

abustamam commented 6 years ago

Thanks for the reply. Yes, you're right; in the above example, it would generate the following CSS:

.glam-1234 .DateRangePicker {
  margin-right: 10px;
}

.glam-1234 .DateRangePicker .CalendarDay {
  border: none;
}

.glam-1234 .DateRangePicker .CalendarDay__selected {
  background-color: blue;
}

.glam-1234 .DateRangePicker .CalendarDay__selected_span {
  background-color: teal;
}

the .glam-1234 is the randomly generated style for the Div component, so it's like writing this:

<div className="glam-1234">
  <DateRangePicker {...props} />
</div> 

Keep in mind that DateRangePicker is a React component exported from an npm library (in this case, I'm using cljsjs to port it over, but I think the concept would be the same even if it were a reagent component). As such, I don't have control over the sub-components/classnames that DateRangePicker would create.

I believe this is the same as attaching styles to child elements in HTML tags with stylefy

Could you clarify how one could do that? I read the snippet on sub-styles:

(def list-container-style (merge generic-container
                                 {::stylefy/sub-styles {:list {:margin-top "1em"}
                                                        :list-item {:color "black"}}}))

(defn list-in-container []
  [:div (use-style list-container-style)
   [:ul (use-sub-style list-container-style :list)
    [:li (use-sub-style list-container-style :list-item) "List element 1"]
    [:li (use-sub-style list-container-style :list-item) "List element 2"]
    [:li (use-sub-style list-container-style :list-item) "List element 3"]]])

It seems to me that you're attaching styles to child elements there, but this is assuming you're in control of the elements. How would you do it when you're not in control of the elements, such as in the Date Range Picker example?

Thanks again!

Jarzka commented 6 years ago

Ah yes, now I understand the case: you want to attach styles to 3rd party components that are not directly managed by you. Obviously, this is not possible with the current version of stylefy. If those 3rd party components do not accept style props, it's difficult to stylize them from outside. You basically have to stylize them the way you mentioned: writing CSS selectors manually. Luckily, you can always use manually written CSS code along with stylefy, so this should not be a showstopper. Still, if you have a good idea how we could do this with a future version of stylefy, please share it with us. :)

For me personally, this is one reason why I do not use 3rd party components very often: they never seem to offer the level of configuration I want, whether I have to configure the behavior of the component or the style of the component. In my own projects, I use Bootstrap to offer me nice HTML+CSS templates, but I always override some of its styles and wrap those templates inside my own React components. This way, I can stylize the components the way I want and create an API for those components that suits my needs. However, I also understand that sometimes this creates duplicate work if you need to wrap HTML+CSS templates to React components manually every time you start a new project and, for some reason, cannot re-use the old code. If there is a 3rd party component that suits your needs, of course you want to use rather than re-implement the wheel.

abustamam commented 6 years ago

Luckily, you can always use manually written CSS code along with stylefy, so this should not be a showstopper.

Yep, I ended up doing that :)

I try not to use 3rd party components as well, but some of them have really great APIs that would be too time consuming to re-implement (like React Dates). I wish I had all the time in the world to make my own component libraries for components I use often, but alas :)

Thanks for the clarification!

Jarzka commented 5 years ago

I have some news related to this issue. Up to this point, the way to override Bootstrap styles with stylefy has been mostly marking some style property as !important. This works, but I believe that generally speaking we should avoid using!important because over usage can easily lead to confusion. Another possible option that was mentioned in this thread was to use custom selector to override the Bootstrap selector. That's kinda what's coming with the next release of stylefy.

The next version is going to contain a manual mode, which basically let's you write custom CSS code within a stylefy style map, using Garden syntax. This custom CSS code can contain the selector you want, and it's scoped inside the element where you use this style map. The point of the manual mode is to be able to resolve some corner cases in which manually written complex CSS selectors are absolutely needed to achieve some result. A simple example of this is a box, which contains another box, and this child box style needs to be changed when the parent box is being hovered. stylefy cannot handle this kind of situation at this point, so we can write our own CSS selector within stylefy to achieve this result. As a bonus, this manual mode also allows us to override Bootstrap styles by writing a selector which is stronger than the Bootsrap selector.

An example of this "box in a box" case can be found from here: https://github.com/Jarzka/stylefy/pull/33/files

For the most part, the introduction of manual mode is not supposed to change how we use stylefy right now (you can keep using !important if you think it's better). In fact, for 98% of the time, we do not need the manual mode at all. But when we need to write custom CSS code, it will now become possible within stylefy.

I would be glad to hear what you think of this.

abustamam commented 5 years ago

Thanks for looking into this @Jarzka ! I think that solution sounds great. I have not been working in CLJS lately but the PR and examples look great.

Jarzka commented 5 years ago

By the way, forgot to mention, that this also makes it possible to modify styles of 3rd party child components that do not take style props as parameters.

Jarzka commented 5 years ago

Manual mode is now live with the release of 1.11.0.

fwaddle commented 4 years ago

Manual mode is now live with the release of 1.11.0.

This is awesome - I finally got around to needing this feature again. The manual mode worked flawlessly and is much better than the other solutions. Virtual high five @Jarzka

Jarzka commented 1 year ago

If someone is still following this thread, I would like to let you know that there is now a native solution for overriding framework styles in CSS with Cascade Layers: https://developer.chrome.com/blog/cascade-layers/

I believe for now on, this is the way to go to solve this problem. We could put all framework specific styles into "framerwork" layer and our application styles into "application" layer. If there is a style confict between the two layers, the "application" layer will always win. Sounds nice and simple.

Cascade layers could work in stylefy, but preferably we need a support for it in Garden first. I asked them about this: https://github.com/noprompt/garden/issues/205