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

feat(weave): dataset editing UI #3341

Merged
merged 1 commit into from
Jan 23, 2025
Merged

feat(weave): dataset editing UI #3341

merged 1 commit into from
Jan 23, 2025

Conversation

bcsherma
Copy link
Contributor

@bcsherma bcsherma commented Jan 7, 2025

Description

  • Fixes WB-22674

Introduces a UI for editing datasets. When viewing dataset objects in weave, an edit button is now shown in the top right corner of the dataset version page. Clicking this button takes the user into the dataset edit experience.

This editor allows:

  • prepending new rows to a dataset
  • removing rows from a dataset
  • modifying existing values in a dataset

At any in the editing process, as long as edits have been made, the user can click publish in the top right corner of the dataset version page to publish a new version with their changes or click the adjacent cancel button to abandon their changes.

Implementation Notes

  • DatasetEditContextProvider is introduced around the dataset version page and exposes state and callbacks for the editor, including: edited cell values, added row values, removed rows.
  • The EditableDatasetView is now always used to render dataset versions. It is almost identical to implementation in DataTableView.tsx but allows for an edit mode where cells can be added, removed, and edited.
  • DatasetVersionPage is extended with state and components to manage the editor lifecycle (start editing, cancel, publish, confirm).
  • The view only and editable modes both support pagination and share pagination state. Sorting is available only in the view mode. Enabling edit mode will remove custom sort settings and reset the page to 0.

@circle-job-mirror
Copy link

circle-job-mirror bot commented Jan 7, 2025

@bcsherma bcsherma force-pushed the dataset-editor-ui branch 2 times, most recently from d966b60 to 0f15fb0 Compare January 7, 2025 23:07
@bcsherma bcsherma force-pushed the dataset-editor-ui branch 3 times, most recently from 36c6fae to 756fddb Compare January 8, 2025 00:02
Base automatically changed from dataset-component to master January 8, 2025 17:36
@bcsherma bcsherma force-pushed the dataset-editor-ui branch 11 times, most recently from 654fd73 to e892715 Compare January 9, 2025 16:40
@bcsherma bcsherma marked this pull request as ready for review January 9, 2025 16:47
@bcsherma bcsherma force-pushed the dataset-editor-ui branch 6 times, most recently from 922e64f to dd1b090 Compare January 15, 2025 21:26
Copy link
Member

@gtarpenning gtarpenning left a comment

Choose a reason for hiding this comment

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

One small issue I ran into was when clicking on row ref links in the dataset viewer. In prod, the ref payload looks like:
"weave:///griffin_wb/prod-evals-nov/object/Dataset:aTIBCGcJAIEcCeyGFxV0IcKSgeYe1GllqxYitg40gBw/attr/rows/id/WmjDCz5lLX3e5MEbiheS04jqGK0XioX4D9L9HrTDb4M"
while on your branch it has an extra _10 (corresponding to the index) appended to it. The ref_read batch fails.
"weave:///griffin_wb/prod-evals-nov/object/Dataset:aTIBCGcJAIEcCeyGFxV0IcKSgeYe1GllqxYitg40gBw/attr/rows/id/WmjDCz5lLX3e5MEbiheS04jqGK0XioX4D9L9HrTDb4M_10"

clicking-row-links

@bcsherma bcsherma force-pushed the dataset-editor-ui branch 13 times, most recently from 610d138 to daa52d6 Compare January 17, 2025 03:01
@bcsherma bcsherma requested a review from gtarpenning January 17, 2025 16:26
@bcsherma bcsherma merged commit 188d09e into master Jan 23, 2025
129 checks passed
@bcsherma bcsherma deleted the dataset-editor-ui branch January 23, 2025 04:16
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants