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

[table] feat: add Table2 and EditableCell2, use new hotkeys API #4790

Merged
merged 8 commits into from
Jul 7, 2021

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Jul 1, 2021

Fixes #4789

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

  • Abstract out hotkeys interaction logic from Table into a plain class (not a React component) called TableHotkeys
    • We instantiate this class in the Table constructor, and keep it up to date with Table props, state, and its Grid as necessary
  • Create a new component, <Table2>, which copies much of its implementation from Table, but has the enhancement of using <HotkeysTarget2> instead of @HotkeysTarget
    • This new component requires that you configure your application with a <HotkeysProvider> somewhere.
  • Create a new compoennt, <EditableCell2>, which copies much of its implementation from EditableCell, but has the enhancement of using <HotkeysTarget2> instead of @HotkeysTarget
  • Use Table2 and EditableCell2 in all the docs-app examples

Note that #4789 contains some discussion of the tradeoffs of alternative solutions to the problem.

Reviewers should focus on:

  • All the docs examples should work with no regressions

Screenshot

@blueprint-bot
Copy link

DRY more table code, add tests

Previews: documentation | landing | table

@blueprint-bot
Copy link

skip bad tests

Previews: documentation | landing | table

@blueprint-bot
Copy link

[v4] fix(useHotkeys): make hook compatible with SSR

Previews: documentation | landing | table

@blueprint-bot
Copy link

add tabIndex

Previews: documentation | landing | table

@@ -1493,7 +1493,9 @@ describe("<Table>", function (this) {
}
});

describe("Autoscrolling when rows/columns decrease in count or size", () => {
// HACKHACK: these tests were not running their assertions correctly for a while, and when that
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: these tests should get fixed in a future PR

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.

HotkeysProvider does not work properly when Table is rendered on the same page
2 participants