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

Changed uiModes Button to Flexbox #4360

Closed
wants to merge 6 commits into from

Conversation

tridip1931
Copy link
Collaborator

@kepta Can you review this.

@tridip1931 tridip1931 mentioned this pull request Sep 20, 2017
@kepta kepta added the wip Work in progress label Sep 21, 2017
css/80_app.css Outdated
.w4 { width:33.3333%; }
.w5 { width:41.6666%; }
.w6 { width:50.0000%; }
.w7 { width:58.3333%; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need the .wX ? I might be wrong but can't we substitute this with flex-grow / flex-shrink?

Copy link
Collaborator Author

@tridip1931 tridip1931 Sep 21, 2017

Choose a reason for hiding this comment

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

I believe flex-grow/flex-shrink only defines how much space to take in a given estate.
CSS grid helps a lot if we are using lots of widths
I thought we needed width minus the floats but right now I am not using them anywhere. We can delete it. @kepta

Copy link
Member

Choose a reason for hiding this comment

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

I agree we might not need this, but I think the idea is for places where you have several joined things, (like buttons) you want them to try to share equal width.

e.g. for OK/Cancel, they'd have 50% width each, even though those words have very different natural width.

I've found I still need to set flex items with a width in order to get them to layout correctly. flexbox-grow is about how to distribute extra width to contained items, and flexbox-shrink is about how to steal width from them (I think - I might be wrong too).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup, but I feel we can simulate the case of proportions % with flex.
Suppose we want 2 guys to share 50:50, we can use simply use default flex.
if 75:25, flex-grow:3 and other to 1.

@tridip1931 @bhousel Is there a situation you guys can think where width% can't be replaced ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes @bhousel I also faced this issue while working in flexbox for keeping some widths, upon looking for answers in google, I found this article very helpful.
https://css-tricks.com/things-ive-learned-css-grid-layout/

TL;DR We should not depend entirely on flexbox for layout purposes.

How do you feel about this guys? @bhousel @kepta

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tridip1931 css-grid is amazing, but we can't use it yet because of very low browser support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh ya. I will delete those widths. Also will figure out the flex-grow/shrink if I come across.
Do we use flex-grow anywhere in our code right now?

Copy link
Member

@bhousel bhousel Sep 21, 2017

Choose a reason for hiding this comment

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

Do we use flex-grow anywhere in our code right now?

It's part of the shorthand flex: property. The keyboard shortcuts screen is probably the place I've most recently used it:

iD/css/80_app.css

Lines 3419 to 3432 in d9b3b7b

.modal-shortcuts .shortcut-tab {
display: flex;
flex-flow: row wrap;
}
.modal-shortcuts .shortcut-column {
flex: 1 1 50%;
width: 50%;
}
.modal-shortcuts .shortcut-tab-tools .shortcut-column {
flex: 1 1 100%;
width: 100%;
}

(also an example of a place where you need to set a width, so flex can start out knowing what it's supposed to do before growing and shrinking)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @bhousel . Thats very helpful. 👍

@bhousel
Copy link
Member

bhousel commented Nov 1, 2018

closing as stale

@bhousel bhousel closed this Nov 1, 2018
@bhousel bhousel removed performance Optimizing for speed and efficiency wip Work in progress labels Nov 8, 2018
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.

3 participants