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

Visibility mixin #20441

Merged
merged 3 commits into from
Oct 28, 2016
Merged

Visibility mixin #20441

merged 3 commits into from
Oct 28, 2016

Conversation

cortopy
Copy link
Contributor

@cortopy cortopy commented Aug 2, 2016

The docs mention an invisible @mixin which is not present in the source scss files.

This adds a visibility mixins file, and references it in the utilities file.

// Visibility

@mixin invisible {
visibility: hidden !important;

Choose a reason for hiding this comment

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

!important should not be used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This !important was in the original file

@tomlutzenberger
Copy link
Contributor

I'm not sure if it's really necessary to add an extra mixin with only 1 rule.
It would increase the code to maintain and add one more file.
IMHO this is an overkill, the .invisible class would be enough.

@cortopy
Copy link
Contributor Author

cortopy commented Aug 2, 2016

@tomlutzenberger this mixin is mentioned in the docs: http:https://v4-alpha.getbootstrap.com/components/utilities/#invisible-content

The way I'm using it is by creating my own class with the three mixins for an accessible invisible class. I thought the docs made sense, as you may or may not want to hide for screen readers too. Without the @invisible mixin my class doesn't work on absolute positioned elements

.hidden
  @include invisible
  @include sr-only
  @include sr-only-focusable

This may not be a good idea for some reason but then the docs need amending

@mdo mdo added this to the v4.0.0-alpha.6 milestone Oct 28, 2016
@mdo mdo merged commit 8f04299 into twbs:v4-dev Oct 28, 2016
@mdo
Copy link
Member

mdo commented Oct 28, 2016

Thanks!

@mdo mdo mentioned this pull request Oct 28, 2016
mdo added a commit that referenced this pull request Oct 28, 2016
@cortopy cortopy deleted the visibility-mixin branch October 28, 2016 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Alpha 6
Fixed/Merged
Development

Successfully merging this pull request may close these issues.

None yet

5 participants