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

Styling by tagname not class #30

Open
ostrgard opened this issue Mar 18, 2016 · 13 comments
Open

Styling by tagname not class #30

ostrgard opened this issue Mar 18, 2016 · 13 comments

Comments

@ostrgard
Copy link

Hi!

As far as I understand, you're aiming at only having nested media-queries and pseudo selectors, not tags or global class names. How would you go about styling basic HTML elements like body, input, h1, h2, p, strong etc. by their tagname? For instance, the body in the example have the default 8px margin – a sitewide reset would be in order.

I can think of a few ways to achieve this, good or (mostly) bad:

  • Site-wide styling of commonly used basic html elements in a seperate css file (do-able now).
  • Dedicated StyleSheet object with basic html, that is required and set as className everywhere they're used: <h1 className={styles.h1}>hi</h1> (whoa!).
  • Some initializer function that accepts sitewide styling by tagname.
  • Accept :global h1, *h1, h1 (a space) or something else, as the first char(s) in the selector when looping through the object keys, and treat that more or less like the pseudo selector.

This is somehow related to #10 , though I see this as the key issue, and a solution would address that as well. I think @jlfwong described the trade-of quite well:

That definitely was not an intention of the API design, but perhaps is something we should support for the sake of API simplicity.

I hope we find a solution for this. I definitely think this is the most promising way of stylehandling!

@ostrgard ostrgard changed the title Styling by tag not class Styling by tagname not class Mar 18, 2016
@xymostech
Copy link
Contributor

Hello @ostrgard! Thank you for your thoughts, I hadn't considered trying to use Aphrodite for resets. It seems a bit orthogonal to our original intention (writing your styles in JS next to the components that they style), so I'm personally a bit hesitant to add special syntax for something like this.

I was thinking a while back about adding the ability to inject your own CSS for letting people do things that Aphrodite doesn't yet support. I was imagining it for problems like "show an element when its parent is hovered over", but it would probably work for you too. I thought of a syntax like this:

var styles = StyleSheet.create({
    hover: {
        ...
    },

    customCss: (me) => unsafeRawCss`
        ${me} {
            display: none;
        }

        ${styles.hover}:hover > ${me} {
            display: inline;
        }
    `,
});

but you could do global resets or styling tag names with it like:

var styles = StyleSheet.create({
    global: () => unsafeRawCss`
        body {
            margin: 0;
        }

        h1 {
            font-family: "Helvetica Neue";
        }
    `,
});

css(styles.global);

Would something like that solve your problems?

@ostrgard
Copy link
Author

Hi @xymostech ! I can totally see the point in your original intention. I guess I'm looking for a way to completely replace any other way of stylehandling, which would require some way of doing global styling, as well as nested styles for complex UI, like your example with hover states styling children. If you have another suggestion, thoughts or another approach to this problem I'm all ears. I might be blindly focusing on a clean and consistent styling solution.

Can you elaborate on the unsafeRawCss template? I'm curious to know how you consider it unsafe, if it's because it's raw and therefor global, or maybe could allow potential injections. I think I'd rather write these global definitions as plain js objects, instead of like a string, just to keep consistency in the API and use the parse/prefix logic that is already set up (please correct me if I'm wrong here).

Thanks for your reply :)

@cesarandreu
Copy link

I'd love to see unsafeRawCss become a reality.

That would enable you to solve cases where you receive a pre-rendered string of HTML and use dangerouslySetInnerHTML. You want to be able to define a bunch of styles to target the classless elements inside, but it loses a bit of its appeal if you end up having to pull in additional tools.

Of course, you'd really want to avoid it as much as possible, but there are cases where you don't have much choice.

@kentcdodds
Copy link
Contributor

@cesarandreu,

That would enable you to solve cases where you receive a pre-rendered string of HTML and use dangerouslySetInnerHTML.

Just finished preparing this example of that very use case and was going to file another issue when I found this.

I think that with something like aphrodite, we should expose escape hatches for people to do crazy things for use cases like this. And using names like dangerouslySetInnerHTML and unsafeRawCss are good enough to indicate that you shouldn't do this normally :-)

I'd happily contribute something with a bit of direction :-)

@barbalex
Copy link

barbalex commented Jun 5, 2016

I think this feature would also enable styling HTML generated by tools like react-bootstraps ProgressBar (https://react-bootstrap.github.io/components.html#progress), where the divs needing to be styled are created by the tool and so they cannot be styled with aphrodite otherwise.

Example:

<ProgressBar now={60} />

builds something like this:

<div>
  <div class="progress">
    <div class="progress-bar" role="progressbar" aria-valuenow="60" aria-valuemin="0" aria-valuemax="100" style="width: 60%;">
      <span class="sr-only">60% Complete</span>
    </div>
  </div>
</div>

and in order to style this I have been using css in a global css file like this (stylus code):

#menu .progress
  margin-top 3px
  margin-bottom 0
#menu .progress-bar
  text-align left
  padding-left 6px

...or am I missing a better way?

@jlfwong
Copy link
Collaborator

jlfwong commented Jun 7, 2016

@barbalex You're not missing anything, so far as I can tell. If you wish your for your only styling solution to be Aphrodite, you cannot accomplish that easily. Thank you for finding the relevant issue on Aphrodite and contributing your example!

@michaelwarren1106
Copy link

What's the aphrodite approach with shadowDOM looming? All the examples I've seen are with non-shadow elements. With angular2's defaulting to shadowDOM elements, shadowDOM is coming soon.

So, it seems to me that aphrodite not allowing styling by tag name means that there's no approach to external "component global" styles. Not every <a> will need a class on it just to receive basic styles like text-underline:none which should be applied to all anchors.

The current recommendations I've seen have been that "its easy to include an external stylesheet like bootstrap" but that approach doesn't work when shadowDOM is turned on. Since that doesn't work, then, it seems to me that aphrodite cannot be the only way to apply styles to an application. There would always need to be some other method of applying styles from outside the component, and those styles that DO come from outside the component are mostly going to generic tag name styles. Why make a class name you need to apply to all <a> tags individually, when you can just write a single generic tag name selector?

And, furthermore, what benefit is aphrodite providing really, when its ultimately splitting up the methods by which my component is styled into 2 pieces. The whole value prop of aphrodite is that you only need JS to style components, but if you want to have a reset in your app, that's no longer true. And that's a pretty simple/default/common occurrence all over the web.

Imo, in order to be widely adopted by large applications, and applications that need to adhere to global/corporate styles, aphrodite needs an approach to style by tag name in addition to classes everywhere.

@kentcdodds
Copy link
Contributor

Hi @michaelwarren1106,

there's no approach to external "component global" styles.

The words "component" and "global" sound like an oxymoron to me. If you need styles to be global, then don't use aphrodite. Just use regular CSS. IMO, Aphrodite's purpose is for styling components. You might think of <a> as a component, and maybe it is. If an <a> is a component, then make an actual component out of it and style it, then use that throughout your application (rather than <a>). Otherwise, just style it and move on.

In general, I find that you really can't (shouldn't) use only aphrodite in your app. You're definitely going to want to use regular CSS for some things (like a reset stylesheet for example).

in order to be widely adopted by large applications, and applications that need to adhere to global/corporate styles, aphrodite needs an approach to style by tag name in addition to classes everywhere.

Again, I think that is out of scope of aphrodite. Aphrodite is intended to style components. Styling things globally is out of scope of aphrodite (and generally not a good idea anyway).

@brad-decker
Copy link

@kentcdodds one of the primary reasons why i'd like to be able to at least style with nested tags is for generated content. Say from Remarkable where you're getting a string of markdown from an api and rendering it to html with a component. Rather than parsing the resulting html and adding classNames it would be very beneficial to be able to define tags contained within a parent aphrodite class in order to style generated content.

Theoretically one could use custom remarkable rules to add classes to every element but this seems rather messy.

@kentcdodds
Copy link
Contributor

Right, I've had this exact same use case. And for me, I've just used normal CSS for styling that. It's pretty minimal most of the time. But if you want to use aphrodite for this, then see what you can do to help get this merged and you can do that and more :)

@kevinSuttle
Copy link

kevinSuttle commented Oct 11, 2016

@kentcdodds I'm thinking UI libs probably need to start looking at CSS Custom Properties. facebook/react#6411 (comment)

@joeshub
Copy link

joeshub commented Mar 11, 2017

Just checking in. Is this still a no go for Aphrodite? I feel like there are 2 separate points here.

  1. Style global things for resets or styleguides for which the unsafeRawCss would be perfect.
  2. Override third party className rules. (Rendered content @kentcdodds, Bootstrap progressbar @barbalex).

For reference:
Styled Components provides a non recommended injectGlobal
Radium has a Style component option
Styletron only lets you pass third party overrides I really like this approach.

Side note: I find it ironic that we are running away from globals and still need them. This third party library stuff will eventually fix itself as people start moving towards css variables though.

@justinwilson-apollidon
Copy link

justinwilson-apollidon commented Nov 21, 2019

+1 for this. Imagine using a React package such as react-paginate where it accepts props such as activeClassName (applied to an a tag) or pageClassName (applied to an li tag). These class props are applied by react-paginate internally. This makes it difficult to use Aphrodite with other packages like this.

I think unsafeRawCss would be a solution that could be used to achieve this though!

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