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

Use flexbox to layout list #75

Merged
merged 4 commits into from
Dec 1, 2016
Merged

Use flexbox to layout list #75

merged 4 commits into from
Dec 1, 2016

Conversation

quincyle
Copy link
Contributor

@quincyle quincyle commented Nov 30, 2016

This project uses semver, please check the scope of this pr:

  • #patch# - backwards-compatible bug fix
  • #minor# - adding functionality in a backwards-compatible manner
  • #major# - incompatible API change

CHANGELOG

  • Updated list layout strategy to use flexbox.

@sglanzer-deprecated
Copy link
Contributor

This is fine, but I'm a little wary about making it a patch - the switch to 100% height on the list may burn some downstream consumers that based their CSS on the previous setup. Can we get confirmation of the impact of this on one of our major apps? If it's no impact, then go with the fix, otherwise go major

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1b41b72 on quincyle:develop into ** on ciena-frost:master**.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1b41b72 on quincyle:develop into ** on ciena-frost:master**.

@quincyle
Copy link
Contributor Author

@sglanzer You're right, it might have some unexpected impact on our downstream apps. Actually, this 100% on list root is not related the flexbox change, it is just something I feel we're missing previously (a default height for list), so I added it. I can just take it out for now and address later.

@coveralls
Copy link

coveralls commented Nov 30, 2016

Coverage Status

Changes Unknown when pulling 1b41b72 on quincyle:develop into ** on ciena-frost:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 27241a9 on quincyle:develop into ** on ciena-frost:master**.

1 similar comment
@coveralls
Copy link

coveralls commented Nov 30, 2016

Coverage Status

Changes Unknown when pulling 27241a9 on quincyle:develop into ** on ciena-frost:master**.

@coveralls
Copy link

coveralls commented Dec 1, 2016

Coverage Status

Changes Unknown when pulling 42c77c2 on quincyle:develop into ** on ciena-frost:master**.

@sglanzer-deprecated
Copy link
Contributor

sglanzer-deprecated commented Dec 1, 2016

Approved

Approved with PullApprove

@sglanzer-deprecated sglanzer-deprecated merged commit b271d5f into ciena-frost:master Dec 1, 2016
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.

None yet

6 participants