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

Add a state field to create utility pseudo-classes #31643

Merged
merged 2 commits into from
Nov 13, 2020

Conversation

Psixodelik
Copy link
Contributor

@Psixodelik Psixodelik commented Sep 14, 2020

Fixes #31639

Added state option for utilities. This will generate custom pseudo-classes when generating utilities.

Example

SCSS

$utilities: (
  "shadow": (
    property: box-shadow,
    state: hover focus,
    class: shadow,
    values: (
      null: $box-shadow,
      sm: $box-shadow-sm,
      lg: $box-shadow-lg,
      none: none,
    )
  )
);

CSS output

.shadow-hover:hover {
  box-shadow: 0 0.5rem 1rem rgba(0, 0, 0, 0.15) !important;
}

.shadow-focus:focus {
  box-shadow: 0 0.5rem 1rem rgba(0, 0, 0, 0.15) !important;
}

.shadow-sm-hover:hover {
  box-shadow: 0 0.125rem 0.25rem rgba(0, 0, 0, 0.075) !important;
}

.shadow-sm-focus:focus {
  box-shadow: 0 0.125rem 0.25rem rgba(0, 0, 0, 0.075) !important;
}

.shadow-lg-hover:hover {
  box-shadow: 0 1rem 3rem rgba(0, 0, 0, 0.175) !important;
}

.shadow-lg-focus:focus {
  box-shadow: 0 1rem 3rem rgba(0, 0, 0, 0.175) !important;
}

.shadow-none-hover:hover {
  box-shadow: none !important;
}

.shadow-none-focus:focus {
  box-shadow: none !important;
}

scss/_utilities.scss Outdated Show resolved Hide resolved
@patrickhlauke
Copy link
Member

personally, not a fan of bloating the CSS with these sorts of pervasive utility classes

@Psixodelik
Copy link
Contributor Author

Psixodelik commented Sep 14, 2020

personally, not a fan of bloating the CSS with these sorts of pervasive utility classes

Hi

I proceed from the fact that I have often seen and used such classes when creating sites. This is common since most designers like to use hover effects. For example, on the card component

If necessary, they can not be generated, as is done by default.

@ffoodd
Copy link
Member

ffoodd commented Sep 14, 2020

Not sure about this either. It has very low value without transition, and this would massively increase the file size when enabled.

@twbs/css-review this (and other PR adding utilities out there) makes me wonder if we shouldn't publish our utility API as an external project —just like RFS— to allow anyone to easily build their own atomic CSS framework, and keep Bootstrap utilities in finer control.

Any opinions?

@MartijnCuppens
Copy link
Member

I'm ok with this PR since it's not disabled by default and could be useful in some cases.

and this would massively increase the file size when enabled

When enabling $enable-hover-utilites, it's only added to some properties which don't have responsive variants, so I guess the difference won't be that huge.

I would personally not enable it on the rounded utilities, I can't think of any use cases for that. Also, this needs documentation.

publish our utility API as an external project

I'm cool with that. The only downside is that we currently copy paste updates since we do not want to rely on package managers like npm. This could potentially be a PITA in the long run.

@Psixodelik
Copy link
Contributor Author

Psixodelik commented Sep 14, 2020

@MartijnCuppens, thanks for the comment

would personally not enable it on the rounded utilities, I can't think of any use cases for that.

Yes you are right. Submitted a new commit

Also, this needs documentation.

Is this worth adding to the theming section of the documentation?

@MartijnCuppens
Copy link
Member

@Psixodelik,

I would like to have some input from @mdo before you continue with this. We haven't really decided (or even discussed) what functionalities we're going to add to our new utility API. We could for example also add focus, dark mode utilities or even RTL utilities.

@mdo, what do you think?

@Psixodelik
Copy link
Contributor Author

@MartijnCuppens

Ok :). I added to the documentation for an example. Thanks

@Psixodelik
Copy link
Contributor Author

@MartijnCuppens

If I come in handy in development, I am always ready to help

@ffoodd
Copy link
Member

ffoodd commented Sep 14, 2020

FWIW my RTL branch extends the API a bit, to prevent some utilities from being generated in RTL output (.text-break mostly). Only used a boolean property: rtl: false that should be set explicitly.

Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be pretty cool. Originally expected to be against this, but given it's opt-in, that seems like a powerful update for folks that want it. Leaving suggestions here that I'll batch merge after submitting this review to fix the typos.

Do we need any other controls here? Do we need a global option at all? Feels like we could skip that entirely and just have the option there for the API.

site/content/docs/5.0/utilities/api.md Outdated Show resolved Hide resolved
site/content/docs/5.0/customize/options.md Outdated Show resolved Hide resolved
scss/_variables.scss Outdated Show resolved Hide resolved
scss/_utilities.scss Outdated Show resolved Hide resolved
scss/_utilities.scss Outdated Show resolved Hide resolved
scss/_utilities.scss Outdated Show resolved Hide resolved
scss/_utilities.scss Outdated Show resolved Hide resolved
scss/_utilities.scss Outdated Show resolved Hide resolved
scss/_utilities.scss Outdated Show resolved Hide resolved
site/content/docs/5.0/utilities/api.md Outdated Show resolved Hide resolved
@Psixodelik
Copy link
Contributor Author

@mdo

Thanks for fix the typos ❤️

@Psixodelik
Copy link
Contributor Author

@mdo Hi, Any news on this PR?

@mdo
Copy link
Member

mdo commented Oct 11, 2020

@Psixodelik Would you be willing to revamp this to support multiple states that users could specify in a list? For example, hover, focus, active for :hover, :focus, and :active? We've had some discussion in other issues where it could be nice to enable folks to extend any utility into various pseudo-classes.

@Psixodelik
Copy link
Contributor Author

@mdo Sure. Today I will refactor the code and add the ability to specify various pseudo-classes

@Psixodelik
Copy link
Contributor Author

Psixodelik commented Oct 11, 2020

@mdo Sorry, I forgot about your commit and made a force push. But everything changed there :D Maybe need squash commit?

@Psixodelik Psixodelik changed the title Add hover utilities Add a state field to create utility pseudo-classes Oct 12, 2020
scss/mixins/_utilities.scss Show resolved Hide resolved
site/content/docs/5.0/utilities/api.md Outdated Show resolved Hide resolved
@Psixodelik Psixodelik requested a review from mdo October 23, 2020 10:22
@Psixodelik
Copy link
Contributor Author

@MartijnCuppens anything to fix here?

@Psixodelik
Copy link
Contributor Author

@mdo Mark, maybe add another transition field to the API? This will make hover and focus less sharp.

It might look like this:

$utilities: (
  "shadow": (
    property: box-shadow,
    state: hover focus,
    transition: 0.7s,
    class: shadow,
    values: (
      null: $box-shadow,
      sm: $box-shadow-sm,
      lg: $box-shadow-lg,
      none: none,
    )
  )
);

CSS output:

.shadow-sm {
  box-shadow: 0 0.125rem 0.25rem rgba(0, 0, 0, 0.075) !important;

  transition: 0.7s;
}

@mdo
Copy link
Member

mdo commented Nov 2, 2020

I don't think we need to add transitions. Utilities are inherently meant to be single property-value pairs.

@devhoussam
Copy link
Contributor

Does it support all [Pseudo-Classes](https://developer.mozilla.org/en-US/docs/Web/CSS/Pseudo-classes)

I think that in Utilities map should make some simple changes something like this :

remove the default responsive option and add it to states option to be like this :

$utilities: () !default;
$utilities: map-merge(
  (
	"text-decoration": (
		states: ('responsive', 'hover', 'focus'),
		property: text-decoration,
		class: text,
		values: none
	),
  ),
  $utilities
);

@mdo
Copy link
Member

mdo commented Nov 9, 2020

Pushed some docs updates here to match the rest of the API page. Care to review @twbs/css-review?

Psixodelik and others added 2 commits November 13, 2020 20:03
remome several hobers

refactoring hover utilities

refactoring hover utilities
Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work fine! I've rebased this PR and squashed some commits :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature][v5] Hover-* class Utilities
8 participants