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

fix(Columns): Add Columns and Column components #101

Merged
merged 23 commits into from
Feb 25, 2019
Merged

Conversation

cm-seekasia
Copy link
Contributor

@cm-seekasia cm-seekasia commented Feb 21, 2019

Example usage:

<Columns>
  <Column>...</Column>
  <Column>...</Column>
  <Column>...</Column>
</Columns>

@markdalgleish markdalgleish changed the title Add Columns component fix: Add Columns component Feb 21, 2019
@cm-seekasia cm-seekasia requested a review from a team as a code owner February 22, 2019 04:32
@markdalgleish
Copy link
Member

Just pushed a big change to the API, to future proof things a bit. We now have a matching <Column> component, rather than letting <Columns> accept arbitrary children.

For example:

<Columns>
  <Column>...</Column>
  <Column>...</Column>
  <Column>...</Column>
</Columns>

This allows us to add props to each Column in the future, e.g. customising the size of the column.

@markdalgleish markdalgleish changed the title fix: Add Columns component fix(Columns): Add Columns and Column components Feb 22, 2019
'.column': {
flexBasis: 0,
flexGrow: 1,
minWidth: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems flexBasis work as the arbitrary minWidth under flexbox 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this was a lesson we learnt from the seek-style-guide about how min-width is handled by flex elements.

@markdalgleish markdalgleish merged commit c588253 into master Feb 25, 2019
@markdalgleish markdalgleish deleted the column branch February 25, 2019 03:52
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.

4 participants