Skip to content

spread headerCellProps into cellProps to use them #27

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

keremciu
Copy link

@keremciu keremciu commented May 8, 2019

I think headerCellProps would be useful to make headerCell. I tried to pass a variable without touching my column object, it's not possible right now.

I think it would be nice to pass all cellProps to cell functions.

I think headerCellProps would be useful to make headerCell. I tried to pass a variable without touching my column object, it's not possible right now.

I think it would be nice to pass all cellProps to cell functions.
@@ -353,7 +353,7 @@ class BaseTable extends React.PureComponent {
const TableHeaderCell = this._getComponent('TableHeaderCell');
const SortIndicator = this._getComponent('SortIndicator');

const cellProps = { columns, column, columnIndex, headerIndex, container: this };
const cellProps = { ...headerCellProps, columns, column, columnIndex, headerIndex, container: this };
Copy link
Contributor

Choose a reason for hiding this comment

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

headerCellProps could be a function see https://github.com/Autodesk/react-base-table/blob/master/src/BaseTable.js#L372, so we can't just spread it, also it's designed for the cell's container, not the user's cell itself, see https://github.com/Autodesk/react-base-table/blob/master/src/BaseTable.js#L380, so it shouldn't be passed to the renderer

@nihgwu
Copy link
Contributor

nihgwu commented May 9, 2019

Hi @keremciu , thanks for your contribution, it's recommended to pass your variables or state to the column definition, or perhaps your cells won't be updated because of the internal optimization

It would be great if you could share some detail of your scenario then I'll try to figure out if there are other ways to achieve that

@keremciu
Copy link
Author

keremciu commented May 9, 2019

hello @nihgwu oh thanks for the feedback, it's so useful, I didn't get the cell's container part. yep basically the scenario is:

I put my own headerCell and cell components into table.

<BaseTable
  {...props}
  components={{
    TableCell: Cell,
    TableHeaderCell: HeaderCell
  }}
/>

I created a util folder called renderers.

it's like ->

  • renderers
    -- string
    -- checkbox
    -- contact
    -- cta

I put format: "checkbox" inside column to call one of these renderers. I'm using same renderers inside many tables.

I wanted to pass general headerProps, rowProps[tableProps] object inside my renderers then I don't need to pass them into every column. Passing into every column is the solution for now. I'm not sure about calling this generic renderers inside column.cellRenderer prop.

@nihgwu
Copy link
Contributor

nihgwu commented May 9, 2019

I just updated the components demo https://autodesk.github.io/react-base-table/examples/components, does that makes sense to you?

The benefit is that the renderers could be shared and used directly via cellRenderer without format, and in the time you could still use cellRenderer to override the result by the format renderer

@keremciu
Copy link
Author

keremciu commented May 9, 2019

@nihgwu oh thank you so much, you're great! it's the same approach which I did. The case was I wanted to pass a general prop to renderers.

as an example: I needed to call "onRowSelect" function inside renderer, I passed onRowSelect key inside column object then I used that prop inside renderer then I realised I've some props I'm passing to many columns then I wanted to pass them directly to the table then use them inside renderers without touching column object.

@nihgwu
Copy link
Contributor

nihgwu commented May 9, 2019

Maybe it would be related to #11 ?
Not sure why you need the onRowSelect callback in the cellRenderer, I think selectedRowKeys would be enough, and update the state, that’s what we are doing internally
But I think you could access anything via the container from the renderer callback and it the ref of the table itself, I’d say that’s a bit hacky and I don’t recommend it
I think we could use Context to achieve your needs, and that’s also my current answer to #11 , maybe I would add something like cellState

@keremciu
Copy link
Author

keremciu commented May 9, 2019

@nihgwu yes, that's the best case passing them as context to use them inside. it could be maybe theme related props? it would be really useful.

I think I need to explain again this onRowSelect stuff.

It's like table based object we can pass to the table as prop. It could extraRenderProps then I can put some props there like { theme, onRowSelect, onAllRowSelect... } then everyone can put their global stuff there. I understand if you don't wanna add a prop as extraRenderProps cause it could maybe change the context of renderers.

I'm gonna close the pull request but it's a really good talk, I hope we can find a solution for #11 and this case

@keremciu keremciu closed this May 9, 2019
@keremciu keremciu deleted the patch-1 branch May 9, 2019 13:43
@nihgwu
Copy link
Contributor

nihgwu commented May 9, 2019

I'm on my home when I post the above comment, I'd like to explain a bit
you could create the context to hold those props and consume them in custom renderers, and that's my current thoughts on #11

<TableContext.Provider>
  <BaseTable />
</TableContext.Provider>

and use it in renderers

renderer = props => (
  <TableContext.Consume>
    {context => <Cell {...props} />}
  </TableContext.Consume>
)

also for the other approach I mentioned above, just put everything you want to the table

<BaseTable renderers={} />

and use in renderers

renderer = ({ container, ...rest }) => {
  const { renderers } = container.props
  return <Cell {...rest} />
}

@nihgwu
Copy link
Contributor

nihgwu commented May 9, 2019

I designed the component for flexibility. Looking forward your use case
BTW I posted a screenshot of our internal table component built on it today https://github.com/Autodesk/react-base-table#example, I'll try to shift more features to this repo, either demos or snippets to show it's potential

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants