Skip to content

Add Table#8

Open
haoxins wants to merge 2 commits intosamrocksc:masterfrom
haoxins:add-table
Open

Add Table#8
haoxins wants to merge 2 commits intosamrocksc:masterfrom
haoxins:add-table

Conversation

@haoxins
Copy link
Copy Markdown
Contributor

@haoxins haoxins commented Sep 19, 2015

No description provided.

src/table.js Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

doesn't upgradeElement need something like 'MaterialTable'?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes. It's MaterialDataTable

@phated
Copy link
Copy Markdown

phated commented Sep 21, 2015

Maybe this should have separate elements, like suggested in the Tabs PR. It doesn't do anything special with the insides of the table, so maybe we can just inject children and the end user can put normal table contents in it. That way you just replace <table> with <Table> and get material styling on it. Thoughts?

@RangerMauve
Copy link
Copy Markdown

Material-ui originally went with a Table component that just had like over nine thousand configuration options for their initial version. However they split that up into a bunch of components that represent parts of a table (TableBody, TableRow, etc).

@haoxins
Copy link
Copy Markdown
Contributor Author

haoxins commented Sep 22, 2015

That way you just replace <table> with <Table> and get material styling on it.

There are some optional classes on table data cells, such as mdl-data-table__cell--non-numeric.
And the user could pass ReactElement as columns and rows also.

@phated
Copy link
Copy Markdown

phated commented Sep 22, 2015

@coderhaoxin I think we should have separate components for rows, columns, etc

@haoxins
Copy link
Copy Markdown
Contributor Author

haoxins commented Sep 23, 2015

@phated

have separate components for rows, columns

done

@phated
Copy link
Copy Markdown

phated commented Oct 2, 2015

I think the mapping needs to go away. A component library should just accept children, not custom DSLs of arrays of objects.

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