deephaven / web-client-ui

Deephaven Web Client UI
Apache License 2.0
28 stars 30 forks source link

fix: DH-17242: Partition Table Pickers not partitioning table properly #2096

Closed AkshatJawne closed 1 month ago

AkshatJawne commented 1 month ago

Resolves https://github.com/deephaven/web-client-ui/issues/2066

This PR is solely to solve the bugs of the partition table; first that the widget was not appearing, and once appearing, the partition selectors not working properly. The PartitionedTable design changes and additional functionality specified in https://github.com/deephaven/web-client-ui/issues/2079 to be brought up here https://github.com/deephaven/web-client-ui/pull/2110

mattrunyon commented 1 month ago

I think your e2e tests might be failing due to a core change. Looks like it's failing to apply date filters

AkshatJawne commented 1 month ago

Hmm interesting, I guess I will try and pull the newest version of main?

AkshatJawne commented 1 month ago

I do see that is due to a screenshot, but I thought that it may be due to the change in the Partition Table label that I made here

mattrunyon commented 1 month ago

Also, if this is the PR that fixes the linked ticket (there are 2 and I think this is the real one as the other PR just includes these commits right now) then change the title to fix: DH-17242: Partition.... That way when we update the web package versions in enterprise, we see the Jira ticket in the changelog and know to update its status

AkshatJawne commented 1 month ago

Will hold off on merging until we can add filter prop to the Picker component, to avoid having to set the viewport on each render to 0,50 -- @bmingles mentioned that will have to wait for #2077 to merge before making these changes.

AkshatJawne commented 1 month ago

Closing this PR given that we are no longer opting to manually set the viewport to 0,50 -- and we ultimately did not add a filter prop to Picker, but rather changed how the options are populated in the pickers.

Refer to https://github.com/deephaven/web-client-ui/pull/2110 for all details and updated changes