Refactor cell display properties and UI

TorreyBetts · TorreyBetts · commit 0380d54bffb2 · 2026-05-11T19:58:23.000-04:00
Add notebook-change handling and refactor display properties. CellPropertiesPanel now subscribes to OnNotebookChanged and invokes LoadSectionsAsync via a new HandleNotebookChanged to refresh when the notebook changes (also fixes subscription/unsubscription scoping). Cell.razor removes the hardcoded "Export" prefix from action button titles. PropertyFieldComponent improves checkbox handling by using IsToggleChecked to support bool, JsonElement and string boolean values. CellDisplayPropertyProvider now builds the fields list incrementally, only exposes input-collapse-related fields for code cells (SupportsInputCollapse), and simplifies preview-style normalization/persistence. A unit test was added to ensure input fields are omitted for non-code cells.
diff --git a/src/Verso.Blazor.Shared/Components/Notebook/Cell.razor b/src/Verso.Blazor.Shared/Components/Notebook/Cell.razor
@@ -98,7 +98,7 @@
                     var act = action;
                     <button class="verso-cell-btn verso-cell-btn--action"
                             @onclick="() => ExecuteCellActionAsync(act)"
-                            title="Export @act.DisplayName">
+                            title="@act.DisplayName">
                         @if (act.Icon is not null)
                         {
                             @((MarkupString)act.Icon)
diff --git a/src/Verso.Blazor.Shared/Components/Notebook/CellPropertiesPanel.razor b/src/Verso.Blazor.Shared/Components/Notebook/CellPropertiesPanel.razor
@@ -51,9 +51,13 @@
         if (_subscribedService != Service)
         {
             if (_subscribedService is not null)
+            {
                 _subscribedService.OnCellExecuted -= HandleCellChanged;
+                _subscribedService.OnNotebookChanged -= HandleNotebookChanged;
+            }
             _subscribedService = Service;
             Service.OnCellExecuted += HandleCellChanged;
+            Service.OnNotebookChanged += HandleNotebookChanged;
         }
 
         if (SelectedCellId != _lastLoadedCellId)
@@ -107,9 +111,21 @@
         });
     }
 
+    private async void HandleNotebookChanged()
+    {
+        await InvokeAsync(async () =>
+        {
+            await LoadSectionsAsync(showLoading: false);
+            StateHasChanged();
+        });
+    }
+
     public void Dispose()
     {
         if (_subscribedService is not null)
+        {
             _subscribedService.OnCellExecuted -= HandleCellChanged;
+            _subscribedService.OnNotebookChanged -= HandleNotebookChanged;
+        }
     }
 }
diff --git a/src/Verso.Blazor.Shared/Components/Notebook/PropertyFieldComponent.razor b/src/Verso.Blazor.Shared/Components/Notebook/PropertyFieldComponent.razor
@@ -24,7 +24,7 @@
         case PropertyFieldType.Toggle:
             <input type="checkbox"
                    class="verso-properties-toggle"
-                   checked="@(Field.CurrentValue is true)"
+                   checked="@IsToggleChecked()"
                    disabled="@Field.IsReadOnly"
                    @onchange="e => OnValueChanged.InvokeAsync(e.Value)" />
             break;
@@ -154,6 +154,15 @@
         StateHasChanged();
     }
 
+    private bool IsToggleChecked() => Field.CurrentValue switch
+    {
+        bool b => b,
+        JsonElement { ValueKind: JsonValueKind.True } => true,
+        JsonElement { ValueKind: JsonValueKind.False } => false,
+        string s when bool.TryParse(s, out var parsed) => parsed,
+        _ => false
+    };
+
     private bool IsSelected(string optionValue) =>
         string.Equals(Field.CurrentValue?.ToString(), optionValue, StringComparison.OrdinalIgnoreCase);
 
diff --git a/src/Verso/Extensions/CellDisplayPropertyProvider.cs b/src/Verso/Extensions/CellDisplayPropertyProvider.cs
@@ -21,54 +21,68 @@ public sealed class CellDisplayPropertyProvider : ICellPropertyProvider
 
     public Task<PropertySection> GetPropertiesSectionAsync(CellModel cell, ICellRenderContext context)
     {
-        var fields = new List<PropertyField>
+        var fields = new List<PropertyField>();
+
+        if (SupportsInputCollapse(cell))
         {
-            new(
+            fields.Add(new(
                 CellViewStateMetadata.InputCollapsedProperty,
                 "Collapse input",
                 PropertyFieldType.Toggle,
-                ReadBool(cell, CellViewStateMetadata.InputCollapsedKey)),
-            new(
-                CellViewStateMetadata.OutputVisibilityProperty,
-                "Output",
-                PropertyFieldType.Select,
-                ReadOutputVisibility(cell),
-                Options: new[]
-                {
-                    new PropertyFieldOption(CellViewStateMetadata.OutputExpanded, "Full"),
-                    new PropertyFieldOption(CellViewStateMetadata.OutputPreview, "Preview"),
-                    new PropertyFieldOption(CellViewStateMetadata.OutputHidden, "Hidden"),
-                }),
-            new(
+                ReadBool(cell, CellViewStateMetadata.InputCollapsedKey)));
+        }
+
+        fields.Add(new(
+            CellViewStateMetadata.OutputVisibilityProperty,
+            "Output",
+            PropertyFieldType.Select,
+            ReadOutputVisibility(cell),
+            Options: new[]
+            {
+                new PropertyFieldOption(CellViewStateMetadata.OutputExpanded, "Full"),
+                new PropertyFieldOption(CellViewStateMetadata.OutputPreview, "Preview"),
+                new PropertyFieldOption(CellViewStateMetadata.OutputHidden, "Hidden"),
+            }));
+
+        if (SupportsInputCollapse(cell))
+        {
+            fields.Add(new(
                 CellViewStateMetadata.InputPreviewLineCountProperty,
                 "Input preview lines",
                 PropertyFieldType.Number,
                 ReadPositiveInt(
                     cell,
                     CellViewStateMetadata.InputPreviewLineCountKey,
-                    CellViewStateMetadata.DefaultInputPreviewLineCount)),
-            new(
-                CellViewStateMetadata.OutputPreviewLineCountProperty,
-                "Output preview lines",
-                PropertyFieldType.Number,
-                ReadPositiveInt(
-                    cell,
-                    CellViewStateMetadata.OutputPreviewLineCountKey,
-                    CellViewStateMetadata.DefaultOutputPreviewLineCount)),
-            new(
-                CellViewStateMetadata.PreviewStyleProperty,
-                "Preview style",
-                PropertyFieldType.Select,
-                ReadPreviewStyle(cell),
-                Options: new[]
-                {
-                    new PropertyFieldOption(CellViewStateMetadata.PreviewStyleLines, "Lines"),
-                }),
-        };
+                    CellViewStateMetadata.DefaultInputPreviewLineCount)));
+        }
+
+        fields.Add(new(
+            CellViewStateMetadata.OutputPreviewLineCountProperty,
+            "Output preview lines",
+            PropertyFieldType.Number,
+            ReadPositiveInt(
+                cell,
+                CellViewStateMetadata.OutputPreviewLineCountKey,
+                CellViewStateMetadata.DefaultOutputPreviewLineCount)));
+
+        fields.Add(new(
+            CellViewStateMetadata.PreviewStyleProperty,
+            "Preview style",
+            PropertyFieldType.Select,
+            ReadPreviewStyle(cell),
+            Options: new[]
+            {
+                new PropertyFieldOption(CellViewStateMetadata.PreviewStyleLines, "Lines"),
+            }));
 
         return Task.FromResult(new PropertySection("Display", null, fields));
     }
 
+    // Matches Cell.razor's SupportsInputCollapse: only code cells render the gutter chevron
+    // and the collapsed-source preview, so only code cells should expose the related fields.
+    private static bool SupportsInputCollapse(CellModel cell) =>
+        string.Equals(cell.Type, "code", StringComparison.OrdinalIgnoreCase);
+
     public Task OnPropertyChangedAsync(CellModel cell, string propertyName, object? value, ICellRenderContext context)
     {
         switch (propertyName)
@@ -116,14 +130,9 @@ private static void SetOutputVisibility(CellModel cell, string? value)
 
     private static void SetPreviewStyle(CellModel cell, string? value)
     {
-        var normalized = string.Equals(value, CellViewStateMetadata.PreviewStyleLines, StringComparison.OrdinalIgnoreCase)
-            ? CellViewStateMetadata.PreviewStyleLines
-            : CellViewStateMetadata.PreviewStyleLines;
-
-        if (string.Equals(normalized, CellViewStateMetadata.PreviewStyleLines, StringComparison.Ordinal))
-            cell.Metadata.Remove(CellViewStateMetadata.PreviewStyleKey);
-        else
-            cell.Metadata[CellViewStateMetadata.PreviewStyleKey] = normalized;
+        // TODO: replace with a real normalize-and-persist when a second preview style is added.
+        _ = value;
+        cell.Metadata.Remove(CellViewStateMetadata.PreviewStyleKey);
     }
 
     private static void SetBoolMetadata(CellModel cell, string key, bool value)
@@ -148,10 +157,9 @@ private static string ReadOutputVisibility(CellModel cell) =>
 
     private static string ReadPreviewStyle(CellModel cell)
     {
-        var value = ReadString(cell, CellViewStateMetadata.PreviewStyleKey);
-        return string.Equals(value, CellViewStateMetadata.PreviewStyleLines, StringComparison.OrdinalIgnoreCase)
-            ? CellViewStateMetadata.PreviewStyleLines
-            : CellViewStateMetadata.PreviewStyleLines;
+        // TODO: normalize against the supported set when a second preview style is added.
+        _ = cell;
+        return CellViewStateMetadata.PreviewStyleLines;
     }
 
     private static string NormalizeOutputVisibility(string? value)
diff --git a/tests/Verso.Tests/Extensions/CellDisplayPropertyProviderTests.cs b/tests/Verso.Tests/Extensions/CellDisplayPropertyProviderTests.cs
@@ -41,6 +41,22 @@ public async Task GetPropertiesSection_ReturnsDisplayFields()
             section.Fields.Select(f => f.Name).ToArray());
     }
 
+    [TestMethod]
+    public async Task GetPropertiesSection_OmitsInputFields_ForNonCodeCells()
+    {
+        var section = await _provider.GetPropertiesSectionAsync(new CellModel { Type = "markdown" }, _renderContext);
+
+        Assert.AreEqual("Display", section.Title);
+        CollectionAssert.AreEqual(
+            new[]
+            {
+                CellViewStateMetadata.OutputVisibilityProperty,
+                CellViewStateMetadata.OutputPreviewLineCountProperty,
+                CellViewStateMetadata.PreviewStyleProperty,
+            },
+            section.Fields.Select(f => f.Name).ToArray());
+    }
+
     [TestMethod]
     public async Task GetPropertiesSection_UsesDefaults_WhenMetadataIsAbsent()
     {

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@`
`98`	`98`	`var act = action;`
`99`	`99`	`<button class="verso-cell-btn verso-cell-btn--action"`
`100`	`100`	`@onclick="() => ExecuteCellActionAsync(act)"`
`101`		`- title="Export @act.DisplayName">`
	`101`	`+ title="@act.DisplayName">`
`102`	`102`	`@if (act.Icon is not null)`
`103`	`103`	`{`
`104`	`104`	`@((MarkupString)act.Icon)`
Original file line number	Diff line number	Diff line change
`@@ -51,9 +51,13 @@`
`51`	`51`	`if (_subscribedService != Service)`
`52`	`52`	`{`
`53`	`53`	`if (_subscribedService is not null)`
	`54`	`+ {`
`54`	`55`	`_subscribedService.OnCellExecuted -= HandleCellChanged;`
	`56`	`+ _subscribedService.OnNotebookChanged -= HandleNotebookChanged;`
	`57`	`+ }`
`55`	`58`	`_subscribedService = Service;`
`56`	`59`	`Service.OnCellExecuted += HandleCellChanged;`
	`60`	`+ Service.OnNotebookChanged += HandleNotebookChanged;`
`57`	`61`	`}`
`58`	`62`
`59`	`63`	`if (SelectedCellId != _lastLoadedCellId)`
`@@ -107,9 +111,21 @@`
`107`	`111`	`});`
`108`	`112`	`}`
`109`	`113`
	`114`	`+ private async void HandleNotebookChanged()`
	`115`	`+ {`
	`116`	`+ await InvokeAsync(async () =>`
	`117`	`+ {`
	`118`	`+ await LoadSectionsAsync(showLoading: false);`
	`119`	`+ StateHasChanged();`
	`120`	`+ });`
	`121`	`+ }`
	`122`	`+`
`110`	`123`	`public void Dispose()`
`111`	`124`	`{`
`112`	`125`	`if (_subscribedService is not null)`
	`126`	`+ {`
`113`	`127`	`_subscribedService.OnCellExecuted -= HandleCellChanged;`
	`128`	`+ _subscribedService.OnNotebookChanged -= HandleNotebookChanged;`
	`129`	`+ }`
`114`	`130`	`}`
`115`	`131`	`}`