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

[core] fix(ControlGroup): normalize fill style #4325

Merged
merged 1 commit into from
Sep 10, 2020

Conversation

ejose19
Copy link
Contributor

@ejose19 ejose19 commented Sep 9, 2020

Fixes #4213

Changes proposed in this pull request:

Normalize fill for ControlGroup like the rest of the components, so it works as intended.

@ejose19
Copy link
Contributor Author

ejose19 commented Sep 9, 2020

Maybe an edit to https://blueprintjs.com/docs/#core/components/control-group.flex-layout is required, as it's stating:

Enable the fill prop on a control group to make all controls expand equally to fill the available space.

However, that was never the case as seen in the docs, using fill on ControlGroup distributes the space proportionally to the original child sizes. True equality can be obtained if all the childs also have fill set.

@adidahiya adidahiya merged commit 4073438 into palantir:develop Sep 10, 2020
@adidahiya
Copy link
Contributor

made a documentation tweak here: 13ff0ae

@ejose19
Copy link
Contributor Author

ejose19 commented Sep 10, 2020

@adidahiya Ok that change is good, but shouldn't the other line be modified too?

from this:

Enable the fill prop on a control group to make all controls expand equally to fill the available space.

to this:

Enable the fill prop on a control group to make all controls fill the available space expanding proportionally to their original sizes.

@adidahiya
Copy link
Contributor

I think the existing documentation can be interpreted as correct; "expand equally" (although somewhat ambiguous) can mean that they grow proportionally, i.e. flex-grow: 1 for all elements

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.

NumericInput fill prop doesn't work
2 participants