enable the minWidth/maxWidth/minHeight/maxHeight features #4183

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
8 participants
@rocman
Contributor

rocman commented Nov 17, 2015

No description provided.

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot Nov 17, 2015

By analyzing the blame information on this pull request, we identified @spicyj, @nicklockwood and @sahrens to be potential reviewers.

By analyzing the blame information on this pull request, we identified @spicyj, @nicklockwood and @sahrens to be potential reviewers.

@nicklockwood

This comment has been minimized.

Show comment
Hide comment
Contributor

nicklockwood commented Nov 17, 2015

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment

@ghost ghost closed this in 2244a86 Nov 17, 2015

calvinf pushed a commit to calvinf/react-native that referenced this pull request Nov 18, 2015

enable the minWidth/maxWidth/minHeight/maxHeight features
Summary: Closes facebook#4183

Reviewed By: svcscm

Differential Revision: D2663931

Pulled By: nicklockwood

fb-gh-sync-id: 53d699fbb6041e3623eb78f1045cac28821efde8
@brentvatne

This comment has been minimized.

Show comment
Hide comment
@brentvatne

brentvatne Nov 22, 2015

Collaborator

We should get this in for Android as well! cc @mkonicek

Collaborator

brentvatne commented Nov 22, 2015

We should get this in for Android as well! cc @mkonicek

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Nov 23, 2015

Contributor

Sorry to be the bad guy here but the implementation of min/max width/height on css-layout isn't ready to be used. It only supports a couple of cases and not even many of the common use cases nor any edge case.

Using this one would be really bad as people are going to rely on this bad implementation and it's going to be a major pain to upgrade to the proper one once we fix the issues

Contributor

vjeux commented Nov 23, 2015

Sorry to be the bad guy here but the implementation of min/max width/height on css-layout isn't ready to be used. It only supports a couple of cases and not even many of the common use cases nor any edge case.

Using this one would be really bad as people are going to rely on this bad implementation and it's going to be a major pain to upgrade to the proper one once we fix the issues

@rocman

This comment has been minimized.

Show comment
Hide comment
@rocman

rocman Nov 23, 2015

Contributor

Concerning about this. Is there any plan or even actual moves on implementing them?

Contributor

rocman commented Nov 23, 2015

Concerning about this. Is there any plan or even actual moves on implementing them?

@nicklockwood

This comment has been minimized.

Show comment
Hide comment
@nicklockwood

nicklockwood Nov 23, 2015

Contributor

@vjeux Is the API likely to change?

Contributor

nicklockwood commented Nov 23, 2015

@vjeux Is the API likely to change?

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Nov 23, 2015

Contributor

@nicklockwood: the api is going to stay the same. But if people use it today and we upgrade it, it'll break their app in subtle ways that we can't make backward compatible.

The current implementation is not just a bit off, it's completely wrong for most common inputs.

Right now no one is working on it, but I'll gladly accept pull requests that improve the implementation.

Contributor

vjeux commented Nov 23, 2015

@nicklockwood: the api is going to stay the same. But if people use it today and we upgrade it, it'll break their app in subtle ways that we can't make backward compatible.

The current implementation is not just a bit off, it's completely wrong for most common inputs.

Right now no one is working on it, but I'll gladly accept pull requests that improve the implementation.

sunnylqm added a commit to sunnylqm/react-native that referenced this pull request Dec 2, 2015

enable the minWidth/maxWidth/minHeight/maxHeight features
Summary: Closes facebook#4183

Reviewed By: svcscm

Differential Revision: D2663931

Pulled By: nicklockwood

fb-gh-sync-id: 53d699fbb6041e3623eb78f1045cac28821efde8

Crash-- added a commit to Crash--/react-native that referenced this pull request Dec 24, 2015

enable the minWidth/maxWidth/minHeight/maxHeight features
Summary: Closes facebook#4183

Reviewed By: svcscm

Differential Revision: D2663931

Pulled By: nicklockwood

fb-gh-sync-id: 53d699fbb6041e3623eb78f1045cac28821efde8
@mkonicek

This comment has been minimized.

Show comment
Hide comment
@mkonicek

mkonicek Feb 5, 2016

Contributor

FYI {min,max}{Width,Height} have been removed from the docs: 2529179

A per vjeux's comment above:

The current implementation is not just a bit off, it's completely wrong for most common inputs. Right now no one is working on it, but I'll gladly accept pull requests that improve the implementation.

The implementation is here: https://github.com/facebook/css-layout

Contributor

mkonicek commented Feb 5, 2016

FYI {min,max}{Width,Height} have been removed from the docs: 2529179

A per vjeux's comment above:

The current implementation is not just a bit off, it's completely wrong for most common inputs. Right now no one is working on it, but I'll gladly accept pull requests that improve the implementation.

The implementation is here: https://github.com/facebook/css-layout

@SudoPlz

This comment has been minimized.

Show comment
Hide comment
Contributor

SudoPlz commented May 18, 2016

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment