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 positions utilities class #19292

Closed
wants to merge 1 commit into from
Closed

Add positions utilities class #19292

wants to merge 1 commit into from

Conversation

CVarisco
Copy link

I apologize if I'm wrong yet. #19289

Utility class features.
Inspiration #18476

Thanks

@cvrebert
Copy link
Collaborator

CC: @mdo for review

@mdo mdo modified the milestone: v4.1 ideas Oct 3, 2016
@mdo
Copy link
Member

mdo commented Jul 2, 2017

These classes are in the wrong place. If you want, please open a new PR against the _position.scss utility file.

@mdo mdo closed this Jul 2, 2017
@mdo mdo removed this from the v4.1 ideas milestone Jul 2, 2017
@CVarisco
Copy link
Author

CVarisco commented Jul 2, 2017

@mdo Sure! I will create a new PR !
Thanks 🙂

@bananatranada
Copy link

@CVarisco @cvrebert @mdo

For completeness, I suggest considering breakpoint infixes as done in _display.scss. Use cases may be rare, but they do exist. It does feel a bit flunky since positioning is heavily reliant on top|right|bottom|left properties, so html and css seem a bit too coupled. What do you guys think?

@bananatranada
Copy link

bananatranada commented Jul 8, 2017

Also to be consistent with _display.scss, I believe it should be .pos-static { position: static; }. It's also more descriptive. Ideally it would just be a p, but that could be mistaken for padding.

@CVarisco
Copy link
Author

CVarisco commented Jul 9, 2017

@bryanbraun
I understand what you mean, I choosed .pos-X to don't write so much.
However, I will wait a little bit to get the best tip.
After that, I will create a PR 🙂

@StephenHogsten
Copy link

This was closed, but it doesn't look like the breakpoints classes were ever added. Was there a reason this was left out, or was it just forgotten (or am I just missing something)

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.

5 participants