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

Decrease number of order utilities #28874

Merged
merged 1 commit into from Jun 18, 2019

Conversation

@MartijnCuppens
Copy link
Member

commented Jun 5, 2019

The list of number of order utilities is huge and I guess it's ok to decrease this list a bit. I've now limited the list to only 5 (+ first/last) responsive order utilities which should be ok

@MartijnCuppens MartijnCuppens requested a review from twbs/css-review as a code owner Jun 5, 2019

@MartijnCuppens MartijnCuppens added this to Inbox in v5 via automation Jun 5, 2019

@mdo

This comment has been minimized.

Copy link
Member

commented Jun 5, 2019

The number was dictated by the column count. Unsure how much we need to really map to that though, so I'd be curious about usage there from people using these utilities.

@MartijnCuppens

This comment has been minimized.

Copy link
Member Author

commented Jun 5, 2019

I personally never used the order classes with numbers, only the first & last utilities to shift some cols for different screen widths. These 5 (+2) order utilities still gives people to completely reorder at least 7 cols which isn't that common I guess.

@mdo

This comment has been minimized.

Copy link
Member

commented Jun 5, 2019

Yeah, it was a holdover from the old push/pull classes in v3. We had all 12 for each direction, and order saved us at least one set of utilities that could produce the same effect. I could see us cutting this in half to start in v5 and see what folks think during the alpha period.

Can you document this in the migration page too then?

@MartijnCuppens

This comment has been minimized.

Copy link
Member Author

commented Jun 5, 2019

@XhmikosR

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

TBH the current way makes more sense due to the number of columns. I guess it's OK, though, if most people don't use more than 5.

@ysds

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

I like this ❤️

Why the last is not 6?

@MartijnCuppens

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2019

Why the last is not 6?

idk, 6 felt a bit too low so I chose 9. Maybe 6 makes more sense indeed.

@MartijnCuppens

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2019

Changed last to 6 now

v5 automation moved this from Inbox to Approved Jun 7, 2019

@ysds
ysds approved these changes Jun 7, 2019

@MartijnCuppens MartijnCuppens requested a review from mdo Jun 7, 2019

@MartijnCuppens

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2019

@mdo, just to be sure, is it ok to merge this for you?

@MartijnCuppens MartijnCuppens force-pushed the mc-master-order branch from 3bbd8ff to 46b2205 Jun 17, 2019

@MartijnCuppens

This comment has been minimized.

Copy link
Member Author

commented Jun 17, 2019

Ping @mdo

@mdo
mdo approved these changes Jun 17, 2019
@mdo

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

Sorry, and thank you!

@MartijnCuppens MartijnCuppens merged commit 83fc5a3 into master Jun 18, 2019

5 checks passed

bundlesize Total bundle size is 226.43KB/235.7KB (-null)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 91.027%
Details
deploy/netlify Deploy preview ready!
Details

v5 automation moved this from Approved to Shipped Jun 18, 2019

@MartijnCuppens MartijnCuppens deleted the mc-master-order branch Jun 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants
You can’t perform that action at this time.