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

Default styling for histoire menu/control tabs being overridden #66

Closed
jfitzsimmons2 opened this issue Apr 12, 2022 · 12 comments · Fixed by #67
Closed

Default styling for histoire menu/control tabs being overridden #66

jfitzsimmons2 opened this issue Apr 12, 2022 · 12 comments · Fixed by #67
Assignees
Labels
bug Something isn't working

Comments

@jfitzsimmons2
Copy link

I'm willing to submit a PR if this is something that the team thinks should be addressed.

In my project, we have a CSS file that is brought in that has same shared styles across components. Currently, when that CSS is brought into histoire, it is overriding the histoire link styles on the sidebar and controls tabs. Is there a way to control the link styles for the sidebar and control tabs? This could be beneficial for future users looking to customize histoire.

In my histoire.setup
import '@path/to/our/library/css.min.css';

This CSS contains a rule
a { color: blue }

Which leads to some unwanted styling side effects within histoire

image
image

Some approaches could be:

  1. Add a tailwind class to try to ensure the links are a certain color
  2. Add a class to the containers for controls/story menus to allow targeting of the links within a custom histoire.css
@hugoattal
Copy link
Collaborator

hugoattal commented Apr 12, 2022

Hum, global CSS in setup file shouldn't leak into the app...

Could you confirm that you use the setupFile like here?
https:/histoire-dev/histoire/blob/main/docs/guide/config.md#global-js-and-css

The import shouldn't occur in the histoire.config.ts but in a separate config file.

EDIT: just to be clear, there are two-way of including CSS into histoire

  • directly import it in the histoire.config.ts file will make the CSS global for histoire
  • import it in a setup file specified in the histoire.config.ts will make the CSS global for stories

EDIT2: Ok, you're right, just tested it and there's something going wrong!

@hugoattal hugoattal added the bug Something isn't working label Apr 12, 2022
@Akryum
Copy link
Member

Akryum commented Apr 12, 2022

CSS can indeed conflict since some stories won't have an iframe isolation (like for the grid layout).

@Akryum
Copy link
Member

Akryum commented Apr 12, 2022

For now I would be in favor of locking down the color with a tailwind class so the design 'just works'

@hugoattal
Copy link
Collaborator

hugoattal commented Apr 12, 2022

Akryum: Yeah, you're right, I can't see an easy solution for this...
Maybe encapsulate all the injected CSS into a class (like .story), but it might be a bit heavy.
Ok for the color lock down with a class.


@jfitzsimmons2 About "[adding] a class to the containers for controls/story":

  • This won't impact the iframe stories since there's no class around it
  • Why not, but what would be your use case? You can add any scoped css in your controls, and even add this class yourself.

@jfitzsimmons2
Copy link
Author

@Akryum 👍 Color lockdown with a tailwind class on the elements in the above screenshot would be more specific than my rule so that would work for me.

@hugoattal My apologies if I am misunderstanding you but the issue I'm describing isn't me wanting to customize a control or a story. The issue is when people bring in their own library styles to histoire.setup.ts, if they have a CSS rule for anchor links it may override the intended look and feel for histoire's story nav and control wrappers (shown in my screenshots above).

@jfitzsimmons2 jfitzsimmons2 changed the title Styling histoire menu/control tabs Default styling for histoire menu/control tabs being overridden Apr 12, 2022
@hugoattal hugoattal self-assigned this Apr 13, 2022
Akryum pushed a commit that referenced this issue Apr 19, 2022
* fix: Lock link color with tailwind class

* typo

* fix cypress test
@tillsanders
Copy link

I'm experiencing the same problem. I think this issue should be reopened as the root cause of the problem remains unsolved.

I'm importing a stylesheet inside my setupFile (not inside histoire.config.js) and I'm using an iframe for my story. However, the style-tags are added both in the iframe and in the root document.

On a side note: maybe a shadow dom can be considered for non-iframe stories.

@Akryum
Copy link
Member

Akryum commented Oct 18, 2022

However, the style-tags are added both in the iframe and in the root document.

This is expected.

@tillsanders
Copy link

Oh, I see. Personally, I wouldn't expect it to be that way. The naming setupFile suggests that it only 'sets the stage' for my story, not for histoire itself. Maybe it would make sense to have separate options for these.

Now, how can I make sure imported styles are not affecting the histoire user interface? Importing inside the story file has the same effect. I also cannot use scoping via selectors (e.g. a global class), because I don't have control over the library I'm importing.

Thank's for your help!

@tillsanders
Copy link

I found a workaround. I noticed that the title of the root document and the iframe document are different. Only the title of the root document contains 'Histoire'. I use that to determine where the setupFile is being executed and only load the css files depending on that condition. This is a little bit of a dirty solution and I think there should be an official way to achieve this. Either with separate files or a global variable or something similar.

// histoire-setup.js
const title = document.querySelector('title')?.text ?? ''
if (!title.includes('Histoire')) {
  // Only executed inside the iframe
  import('mylibrary/myLibrary.css')
}

@Akryum
Copy link
Member

Akryum commented Oct 18, 2022

You workaround will work only if there is an iframe, but it's not always the case (with the grid layout for example).

@tillsanders
Copy link

Yes, that is a limitation. I don't think there is a way around it.

Do you think my general use case / issue should be addressed by a future release?

@tillsanders
Copy link

I noticed that my workaround is only working during dev mode, not in a static build. I tried to use another workaround based on window.location.pathname to determine wether the script is run inside the iframe. For some reason, that, too, doesn't work with a static build.

Right now, I can use setupFile to customize the Histoire interface but I'm risking interfering with my stories. Or I can use setupFile to emulate an environment for my story but then I'm risking interfering with the Histoire interface. To me it looks like Histoire needs a better sandbox that will allow me to inject at least styling only into the story/sandbox. That would mean either using an iframe for every story or using a shadow DOM or a combination. Or am I missing something?

@Akryum You're building an amazing tool here and a great alternative to storybook. Thank you so much for your work. I would love to hear your thoughts on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants