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: Wrap internal utilities into a context provided from HotTable to its children #10789

Conversation

NOtherDev
Copy link

Context

Changing the way HotTable communicates with HotColumns below. Right now, to avoid props passing and expose simple <HotTable ...><HotColumn ... /></HotTable> children-based API, the utilities are shared from HotTable to HotColumn via React.Children iteration and cloneElement "magic". We agreed it's not idiomatic React and React way to do it would be to use context.

However, it turned out that besides shared utils, the HotColumn instances get columnIndex prop which obviously its different for each children, so the only way to do it without forcing end-users to specify these indices is to keep a part of the current approach.

I also skipped moving hotInstance access to the context as right now there's no need to do so. HotInstance is initialized and accessed only from HotTable - moving it elsewhere will only introduce unnecessary complexity.

How has this been tested?

  • All the unit tests still pass. No new unit tests as this is an internal structural change with no intention to be observable from the outside.
  • 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)

☝️ actually, none of them. This is purely a structural change with no behavior change intended.

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 marked this pull request as ready for review February 19, 2024 11:49
@evanSe evanSe merged commit faf0c9f into handsontable:feature/improved-react Feb 19, 2024
16 of 18 checks passed
@NOtherDev NOtherDev deleted the feature/improved-react-2-hottablecontext branch February 19, 2024 14:22
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