Skip to content

Add all missing cursor utilities#674

Closed
hacknug wants to merge 1 commit intotailwindlabs:nextfrom
hacknug:feature/cursors
Closed

Add all missing cursor utilities#674
hacknug wants to merge 1 commit intotailwindlabs:nextfrom
hacknug:feature/cursors

Conversation

@hacknug
Copy link
Copy Markdown
Contributor

@hacknug hacknug commented Feb 21, 2019

This PR adds all of the missing cursor utilities. It also deprecates my plugin [cursor-extended](https://github.com/hacknug/tailwindcss-cursor-extended) 🎉

Closes #638. Closes #658.

@adamwathan
Copy link
Copy Markdown
Member

Ah this is way more than I was expecting when we were chatting about it on Discord (150 new classes by default 😬). I don't mind adding the options from #658 out of the box but don't want to get too carried away. I wonder if it would be better to just make this configurable so you can easily add a missing cursor if you need it, even though you can't actually add custom values...

@hacknug
Copy link
Copy Markdown
Contributor Author

hacknug commented Feb 21, 2019

That's why I was thinking of a special variant called recommended that would simply reduce the amount of classes a given plugin outputs.

Like I mentioned on Discord, such variant could be used for cursor (#658), lists (#275), backgroundRepeat, borderStyle (#611), border (#559 maybe), or even pointerEvents adding support for the SVG-only properties.

We would be able to throw lots of more CSS into core without affecting at all the current generated classes.

@benface
Copy link
Copy Markdown
Contributor

benface commented Feb 21, 2019

Or maybe there could be different base configs to start from and extend? default for the "recommended" utilities and full for ALL of them.

@bradlc
Copy link
Copy Markdown
Contributor

bradlc commented Feb 21, 2019

Or maybe there could be different base configs to start from and extend? default for the "recommended" utilities and full for ALL of them.

What do you think about having an explicit extends key in the config? For example:

module.exports = {
  // `tailwindcss/{default,recommended,full}` etc. or `@mycompany/tailwind`
  extends: require('tailwindcss/default'),
  theme: {
    // ...
  }
}

This has a few benefits I think. One being that it is much more clear that there is a merge going on. I think that the new stripped-down default config could be a source of confusion for people new to Tailwind. Adding extends makes it clear that there’s something going on, and would go a long way to avoiding confusion in my opinion.

It also means you have the choice to opt-out of the merge completely (just remove the extends), or create your own shareable config (e.g. @mycompany/tailwind)

@hacknug
Copy link
Copy Markdown
Contributor Author

hacknug commented Feb 21, 2019

It’s a great idea but it still wouldn’t solve the issue of cursors adding too many utilities that no one would need 99% of the time, right?

@adamwathan
Copy link
Copy Markdown
Member

@bradlc That's pretty interesting! Gonna think harder about that 🤔

I think something like recommended variant or similar is a bit too complicated, I'd rather cursor just be a property in the theme where users can manually add the other ones with whatever names they want if they need them.

@adamwathan
Copy link
Copy Markdown
Member

My only real hesitation with the extends thing is that there's nothing really stopping you from solving it with Just JavaScript™ already:

// Brad's idea
module.exports = {
  extends: require('@mycompany/tailwind-theme'),
  theme: {
    colors: { ... },
    extend: {
      spacing: { ... },
    },
  },
}

// Just JavaScript™
module.exports = {
  theme: require('@mycompany/tailwind-theme')({
    colors: { ... },
    extend: {
      spacing: { ... },
    },
  }),
}

You'd just need to write some merging logic. It's still kinda interesting though, maybe there are other things you could do with the extends approach that make it more useful than trying to solve it in user land with just JS.

Would be interested in discussing the idea in more detail in a separate issue 👍

@hacknug
Copy link
Copy Markdown
Contributor Author

hacknug commented Feb 22, 2019

I'd rather cursor just be a property in the theme where users can manually add the other ones with whatever names they want if they need them.

Yup. I just realized this would be a better option since sometimes you need to create your own cursor and this wouldn't be possible without this being a property of theme.

Will close this PR and open a new one for that.

@hacknug hacknug closed this Feb 22, 2019
@adamwathan
Copy link
Copy Markdown
Member

Oh wow, forgot that you can have custom cursors. That makes it easy for sure 👍 Thanks man!

@hacknug
Copy link
Copy Markdown
Contributor Author

hacknug commented Feb 22, 2019

Yeah, I also didn't thought about it when we discussed it.

The only issue I see with this now is that maybe we should find/define a way to do this? 🤔

.cursor-eye {
  cursor: svg-load('eye.svg', fill=#fff, width=75, height=75) 37.5 37.5, pointer;
  cursor:
    image-set(
      svg-load('eye.svg', fill=#fff, width=75, height=75) 1x,
      svg-load('eye.svg', fill=#fff, width=150, height=150) 2x
    ) 37.5 37.5,
    pointer;
}

@adamwathan
Copy link
Copy Markdown
Member

I think for now we don't need to solve for fallback stuff like that. We could make it possible one day (would just be a new feature not a breaking change anyways), but for now I think it's fine to say "use another PostCSS plugin for that" because you could definitely write something that parses the image-set call and extracts the first option for browsers that don't support image-set.

@adamwathan
Copy link
Copy Markdown
Member

Looks like there's already a plugin that solves this at a general level anyways: https://github.com/jonathantneal/postcss-image-set-function

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

Successfully merging this pull request may close these issues.

4 participants