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

React wrapper: Change the way renderer might be passed to HotColumn and HotTable to use render props approach #10791

Conversation

NOtherDev
Copy link

Context

Specifying renderers as children components marked with "magic" hot-renderer attribute does not feel idiomatic. It doesn't represent something that is a part of the column, it's purely the column's property. Moreover, this kind of composition allows putting multiple Renderers in one column, which is illogical.

Instead, we're proposing to use a variant of render props pattern and allow specifying component-based renderers via HotTable or HotColumn props directly. The awesome part of this approach is we can keep things heavily typed in regards to typescript, it is the preferred approach when it comes to proper react and involves less magic behind the scenes.

How has this been tested?

  • All the unit tests still pass + new behavior tests implemented.
  • Example projects from the repo still work properly.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Additional language file or change to the existing one (translations)

Related issue(s):

  1. Proposal in RFC
  2. Already pre-reviewed discussion with @evanSe in my private fork

Affected project(s):

  • handsontable
  • @handsontable/angular
  • @handsontable/react
  • @handsontable/vue
  • @handsontable/vue3

Checklist:

@NOtherDev NOtherDev changed the title Change the way renderer might be passed to HotColumn and HotTable to use render props approach React wrapper: Change the way renderer might be passed to HotColumn and HotTable to use render props approach Feb 19, 2024
@NOtherDev NOtherDev marked this pull request as ready for review February 19, 2024 15:46
@evanSe
Copy link
Member

evanSe commented Feb 21, 2024

@NOtherDev did you want me to merge this one for you? I also noticed there was one failing React test

@NOtherDev
Copy link
Author

@NOtherDev did you want me to merge this one for you? I also noticed there was one failing React test

Sorry for the test - that was a new one that I missed when merging onto the newest develop. Fixed now.

Yes, this is a PR in handsontable repo already so I don't have permissions to merge it. I assumed you'd merge it into feature/improved-react as you did with #10785 and #10789.

@evanSe evanSe merged commit b0cc886 into handsontable:feature/improved-react Feb 21, 2024
18 of 19 checks passed
@NOtherDev NOtherDev deleted the feature/improved-react-3-hotrenderer branch February 21, 2024 13:28
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.

None yet

2 participants