Skip to content

Persist cell view state and add clear output action#52

Merged
TorreyBetts merged 6 commits into
DataficationSDK:mainfrom
eosfor:pr/cell-view-state-clear-output
May 11, 2026
Merged

Persist cell view state and add clear output action#52
TorreyBetts merged 6 commits into
DataficationSDK:mainfrom
eosfor:pr/cell-view-state-clear-output

Conversation

@eosfor
Copy link
Copy Markdown
Contributor

@eosfor eosfor commented May 6, 2026

Problem

The notebook UI could collapse inputs or change output visibility, but those view choices were not persisted through the notebook model/host round trip. Clearing output also lacked a first-class built-in toolbar action with host-level coverage.

Approach

Persist per-cell view state in cell metadata and expose service methods for updating that state. The host protocol round-trips metadata changes so the UI state survives reloads. This PR also adds a built-in clear output toolbar action and tests the host behavior.

Notable changes

  • Add CellViewStateMetadata keys for input collapse and output visibility.
  • Persist per-cell input collapsed state in cell metadata.
  • Persist per-cell output visibility state in cell metadata.
  • Add host protocol handling for cell metadata updates.
  • Round-trip view metadata through notebook DTOs.
  • Update server and WASM notebook services to update metadata through the appropriate local/remote path.
  • Add a built-in clear output toolbar action.
  • Add host tests for metadata round-tripping and clear output behavior.
  • Update the Blazor shared test fake to support the new cell view state service methods.

Validation

  • dotnet test tests/Verso.Host.Tests/Verso.Host.Tests.csproj --no-restore
  • dotnet build Verso.sln --no-restore

Manual check

Collapse a cell input or change output visibility, save/reopen the notebook, and verify the view state is restored. Trigger the clear output toolbar action and verify the selected cell output is removed.

Review notes

The BuiltInExtensionDiscoveryTests count adjustment from the original branch was not needed on the current upstream base, so the PR contains only the active implementation/test changes required on main.

@eosfor
Copy link
Copy Markdown
Contributor Author

eosfor commented May 7, 2026

Rebased this branch onto current main with -Xrenormalize and force-pushed. GitHub now reports it as mergeable.

@TorreyBetts
Copy link
Copy Markdown
Contributor

Hi @eosfor, when you get the chance, could you sync up your branch? This is the next PR I'll be reviewing and testing. Thanks!

@eosfor eosfor force-pushed the pr/cell-view-state-clear-output branch from fac8e7b to 28ce201 Compare May 9, 2026 06:52
@eosfor
Copy link
Copy Markdown
Contributor Author

eosfor commented May 9, 2026

Synced this branch with current main and force-pushed.

I also fixed the CI failure by updating the built-in extension count test for the new verso.action.clear-cell-output action. The coverage summary step was also guarded so fork PRs do not fail when GitHub blocks the comment permission.

Manual F5 verification passed for input collapse/output visibility persistence and clear output behavior. CI is now green on the latest run.

@TorreyBetts
Copy link
Copy Markdown
Contributor

TorreyBetts commented May 10, 2026

Hi @eosfor. After testing the PR this feels like the right direction. With that said, for the UI, I'd rather route the new settings through ICellPropertyProvider (the same extension point the existing CellVisibilityPropertyProvider uses) instead of adding toolbar pills to every cell. The chevron change to editable code cells works as the toggle and can stay.

A few asks if you're up for the rework:

  • Replace the "Code:" and "Output:" pills with a CellDisplayPropertyProvider that adds a "Display" section to the properties pane. Fields: a checkbox for input collapse, a select for output visibility (Full / Preview / Hidden), and configurable preview line counts (the hardcoded 2L and 5L should be settings). Keep the preview style as a separate select (default "Lines" for the current line-clamp behavior) so future preview styles can be added without growing the visibility enum.
  • Drop the new cell/updateMetadata host RPC. properties/updateProperty plus OnPropertyChangedAsync already covers this.
  • Rename the metadata keys from verso.ui.* to verso:ui.* to match the existing verso:visibility convention.

The chevron change, the persistence, the tests, and ClearCellOutputAction are good as is. The CI fork coverage guard is fine to keep here too.

And if you'd rather hand off the rework, no problem. I can pick it up from your branch and credit your contribution. The persistence groundwork is solid either way.

Note: the title="Export @act.DisplayName" in Cell.razor around line 106 is a bug I found while testing this PR, since ClearCellOutputAction is the first non-export action to render through that. I've tracked that separately and will fix post merge.


In case you or anyone else reading this wants to see what the built-in properties pane field types look like, here's a self contained verso notebook that adds cell properties after creating an extension (through cell):

cell-properties-example.verso.zip

@eosfor
Copy link
Copy Markdown
Contributor Author

eosfor commented May 11, 2026

Updated the branch in 9f8d8bb to address the display-state review feedback.

  • Replaced the Code: / Output: toolbar pills with CellDisplayPropertyProvider and a Display section in the properties pane.
  • Added input collapse, output visibility (Full / Preview / Hidden), input/output preview line counts, and preview style (Lines).
  • Removed cell/updateMetadata; updates now use properties/updateProperty + ICellPropertyProvider.OnPropertyChangedAsync.
  • Renamed metadata keys to verso:ui.*.
  • Kept the editable-code chevron collapse affordance, now backed by the same provider metadata path.
  • Kept ClearCellOutputAction and the fork coverage guard.
  • Added/updated provider, host update, save/open round-trip, and Blazor shared tests.

Validation: CI is green, and F5 manual validation passed in the VS Code Extension Development Host.

image

@TorreyBetts TorreyBetts merged commit 548a4c8 into DataficationSDK:main May 11, 2026
5 checks passed
@TorreyBetts
Copy link
Copy Markdown
Contributor

Thanks for taking the time to rework this. I appreciate the contribution.

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.

2 participants