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

CSS theme causing conflict #3

Open
victornovais opened this issue Jul 1, 2015 · 9 comments
Open

CSS theme causing conflict #3

victornovais opened this issue Jul 1, 2015 · 9 comments

Comments

@victornovais
Copy link

First of all, congrats for the theme, it's very clean and elegant!

Using the theme I realized it's CSS it's not well encapsulated. For example, I was building a bootstrap for a list, and the css of the theme conflicted with my own.

Here's a screenshot:
screenshot from 2015-07-01 15 11 32

How can I fix this?

@hupf
Copy link
Member

hupf commented Jul 2, 2015

Hi @victornovais

Thanks for reporting this. I've improved the encapsulation of the theme styles. Can you test your styleguide using this theme's master? I hope it fixes your issues will release a new version if everything is okay.

Mathis

@ChrisSargent
Copy link

Hi - echoing above, it's a really cool theme and saving me a bunch of time so thanks and congrats!

I also have a small issue with the encapsulation of tags in the content though, I guess caused by this:

.hgt-pagehead-nav a, .hgt-content a, .hgt-footer a {
  color: #4183C4;
  text-decoration: none;
}

.hgt-content a:hover, .hgt-footer a:hover {
  text-decoration: underline;
}

For example if I was using something like:

<div>
    <button class="btn-default">login</button>
    <a href="#" class="btn btn-default">login</a>
</div>

For now, I've just removed the CSS affecting the a tags in .hgt-content as follows:

.hgt-pagehead-nav a, .hgt-footer a {
  color: #4183C4;
  text-decoration: none;
}

.hgt-footer a:hover {
  text-decoration: underline;
}

But if you have a better idea, let me know.

@ChrisSargent
Copy link

Yeah, same issues with fonts etc in the .hgt-content div.

@justinhelmer
Copy link

Although this comment is roughly 4 months old, this issue still exists today. From what I've found, there aren't too many suggestions out there.

Thought I'd throw my hat in the ring.

I see this problem often with hologram themes; that is, issues related to CSS specificity. It can result in:

1) The hologram theme inheriting application styles:

theme-inheriting-from-app

2) The code examples inheriting the hologram theme styles:

app-inheriting-from-theme

Both are bad.

The only way to assuredly avoid this by relying solely on CSS specificity would be to ensure uniqueness of every style definition, both in the hologram theme as well as in the application CSS. This, of course, is not a real solution. Themes can be developed by anyone, who may adhere to whatever style guidelines they desire. Application code needs to consider CSS inheritance among other things for scaling purposes, and can not feasibly specify unique style definitions for every.single.element.

For past projects, my "workaround" was to avoid contributed hologram themes entirely, and use the application's CSS to build out the hologram theme. This alleviates the concern of conflicts, but adds overhead for each new project. Additionally, there are really nice themes that already exist, such as the hologram-github-theme.

Finally, I designed a solution that works consistently, requires little effort, can be reused for all projects, and can be applied to any hologram theme. It guarantees complete isolation between the theme CSS and the application CSS. Sound too good to be true? Then you might laugh at the solution:

1) First, replace the default markup_example_template.html.erb, which looks like this:

<!-- @file markup_example_template.html.erb -->
<div class="codeExample">
  <div class="exampleOutput">
    <%= rendered_example %>
  </div>
  <div class="codeBlock">
    <div class="highlight">
      <pre><%= code_example %></pre>
    </div>
  </div>
</div>

...with a new file by the same name, that uses an iframe's srcdoc attribute to build a completely isolated document context:

<!-- @file markup_example_template.html.erb -->
<div class="codeExample">
  <div class="exampleOutput">
    <iframe srcdoc='
      <head><link rel="stylesheet" href="content/main.css"></head>
      <body><%= rendered_example %></body>
    '></iframe>
  </div>
  <div class="codeBlock">
    <div class="highlight">
      <pre><%= code_example %></pre>
    </div>
  </div>
</div>

You will notice a reference to the app's stylesheet in the <head> section of the isolated document. This only exists because it was copied over as a dependency:

# @file hologram_config.yml
dependencies:
  - ../build/development/content
  • Note that css_include should NOT reference the app's stylesheet, else it will be loaded into the context of the hologram theme.

2) Next, include a script file to handle correctly sizing the iframes:

# @file hologram_config.yml
js_include:
  - support/globals.js
// @file globals.js
window.onload = resizeFrames;

function resizeFrames() {
  var frames = document.getElementsByTagName('iframe');

  // set the height of the iframe to the height of its content
  for (var i = 0; i < frames.length; i++) {
    frames[i].height = 0; // Allows frames to shrink if content height < default frame height
    frames[i].height = frames[i].contentWindow.document.body.scrollHeight + 'px';
  }
}

3) Finally, include a css file to correct the width of the iframe, and make it vanish from sight:

# @file hologram_config.yml
css_include:
  - support/globals.css
/* @file globals.css */
iframe {
  border: 0;
  width: 100%;
}

That's it! The document scope for each individual rendered example block is completely isolated from the hologram theme. There is also the added benefit of the example blocks being isolated from one another. This allows for example-specific CSS that doesn't have to be wildly specific to ensure it doesn't leak over. For example, you might have this as your html_example block:

<div class="display-flex-row justify-content-center align-items-center example">
  <div class="block blue">I am left-of-center</div>
  <div class="block red">I am right-of-center</div>
</div>

<style type="text/css">
  .example {
    height: 200px;
  }

  .block {
    border: 1px solid transparent;
    height: 100px;
    margin: 0 10px;
    padding-top: 20px;
    text-align: center;
    width: 200px;
  }

  .blue {
    border-color: blue;
  }

  .red {
    border-color: red;
  }
</style>

These are class definitions that really only apply to the single example, and can make the example more clear by not having inline styles.

This may not be the most elegant solution, but it gets the job done, and does it well. This should enable everyone to focus on more important problems.

@RolandStuder
Copy link

Hi Justin. Thank you for your detailed input! Looks like a viable solution to me. I will try it out and then discuss with @hupf

@hupf
Copy link
Member

hupf commented Dec 10, 2015

I'm already working on a solution with more specific CSS rules that I'll be able to push when I find some more time. We might also try to approach with the `ìframe``, that sound legit (indeed, some elements are not easily addressable).

@justinhelmer
Copy link

@RolandStuder - Good luck!

@hupf - Thanks for responding. Completely unique style definitions will certainly help mitigate the issue of the code examples inheriting the hologram theme styles, but there is still the issue of the theme inheriting the application styles.

The only way I could find true isolation was with an iframe, and I don't have change how I architect my CSS.

@hupf
Copy link
Member

hupf commented Dec 11, 2015

Alright, I made the css rules more specific, now they should no more conflict with the content css. I think this solution is fine, since the iframe approach is quite hacky.

@justinhelmer: can you test your styleguide with the version on master? I'll create a new release if it's working.

@RolandStuder
Copy link

I understand the iframe solution feels hacky, I would not make it the default, but it could be a viable option. Thanks @hupf for the update.

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

5 participants