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

Styling using Context #49

Open
Chris-Petty opened this issue Jul 23, 2019 · 2 comments
Open

Styling using Context #49

Chris-Petty opened this issue Jul 23, 2019 · 2 comments
Labels
Feature New or distinctly changed functionality work Priority: normal Bug or feature but has work around Pure/Virtualised list rewrite

Comments

@Chris-Petty
Copy link
Contributor

Chris-Petty commented Jul 23, 2019

Is your feature request related to a problem? Please describe.

Need easy top level way of providing styling theme to all components of react-native-data-table library.

Describe the solution you'd like

Use Context API in React, make all components subscribe to it and use it.

Describe alternatives you've considered

Passing through styles through props (give Cells/Rows etc. styles prop):

  • bad: can be annoying and messy to implement. In-line combining styleSheets will probably trigger unwanted shallow compare diffs (re-render hell, react hooks could memoize, maybe).
  • good: does give flexibility to control individual cells.

Maybe sufficient with nesting in styleSheet, though? Perhaps we should do both.

Additional context

Add any other context or screenshots about the feature request here.

@Chris-Petty Chris-Petty added Priority: normal Bug or feature but has work around Feature New or distinctly changed functionality work Pure/Virtualised list rewrite labels Jul 23, 2019
@josh-griffin
Copy link

@Chris-Petty

I experimented a little with passing a style prop to Row and cell components through renderRow and renderCell - (no stylesheet, just an object), didn't trigger any extra re-renders.

Also tried with context (passing a style object), no extra re-renders either.

I like context, I'm not sure if it's overkill or not. I think theming might be though? It does quite seriously limit the flexibility by only providing essentially margin/padding amounts, colours (I think that's what you mean?)

i.e.

{
    colours: {
        CellBorderColor,
        cellBackgroundColor,
        ...
    }
}

maybe a different shape, i.e. cell: {}, row: {}

Or alternatively, provide styles in context

{
    editableCellStyle,
    rowStyle,
    ...
}

Although this also limits possibilities when using variables to determine styles. i.e. alternating rows - pass two row styles and an alternate prop or something like that? seems a lot easier to just pass through renderRow, renderCell - more flexible and easier?

@Chris-Petty
Copy link
Contributor Author

The renderProp pattern (renderRow, renderCells) give plenty of flexibility, but the point of anything here is maybe using context to cut down on certain things like having to give textStyle as a prop to pass through the component in every cell. So long as you manage rowState correctly, which can be challenging or at least quite verbose.

Buuut it's not that onerous really. I'm content with 1-2 props for styling things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New or distinctly changed functionality work Priority: normal Bug or feature but has work around Pure/Virtualised list rewrite
Projects
None yet
Development

No branches or pull requests

2 participants