mjasin/Nexus.Reader
1
0
Fork 0
Code Issues 9 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix(ingest): implement beautiful upload loading state and fix button loading spinner visibility #66

Merged
mjasin merged 4 commits from bugfix/book-upload-indicator into infra/beta-deploy-test 2026-06-01 16:42:09 +00:00
Conversation 2 Commits 4 Files Changed 6
+19 -9
2 changed files with 19 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit f81b2acc40 - Show all commits
src/NexusReader.UI.Shared/Components/Organisms/BookIngestionModal.razor
+11 -4
View File
@@ -33,8 +33,15 @@
<p>Scanning metadata...</p>
</div>
</div>
<div class="ingesting-state shimmer" style="@(IsIngesting && !IsIndexing ? "display:flex;" : "display:none;")">
mjasin marked this conversation as resolved Outdated
Antigravity commented 2026-06-01 16:31:29 +00:00
Outdated
Review
Copy Link
Copy Source

🟡 ingesting-state is hidden by the time IsIndexing could be true — redundant guard

The !IsIndexing condition in the ingesting-state visibility expression is always vacuously true during the visible window of this state. In SaveToLibrary, IsIngesting is set to false before IsIndexing is set to true (lines 309-310), so the two flags are mutually exclusive by construction.

Either:

  1. Add a comment: // !IsIndexing is redundant by construction but kept for defensive consistency with parsing-state
  2. Or remove it: style="@(IsIngesting ? "display:flex;" : "display:none;")"

The current state is not a bug, but it creates a false impression that IsIngesting and IsIndexing can both be true simultaneously.

🟡 **`ingesting-state` is hidden by the time `IsIndexing` could be true — redundant guard** The `!IsIndexing` condition in the `ingesting-state` visibility expression is always vacuously true during the visible window of this state. In `SaveToLibrary`, `IsIngesting` is set to `false` *before* `IsIndexing` is set to `true` (lines 309-310), so the two flags are mutually exclusive by construction. Either: 1. Add a comment: `// !IsIndexing is redundant by construction but kept for defensive consistency with parsing-state` 2. Or remove it: `style="@(IsIngesting ? "display:flex;" : "display:none;")"` The current state is not a bug, but it creates a false impression that `IsIngesting` and `IsIndexing` can both be `true` simultaneously.
<div class="shimmer-content">
<div class="spinner"></div>
<p>Saving book to library...</p>
</div>
</div>
<div class="verification-state" style="@(IsVerifying && !IsParsing && !IsIndexing ? "display:flex;" : "display:none;")">
<div class="verification-state" style="@(IsVerifying && !IsParsing && !IsIndexing && !IsIngesting ? "display:flex;" : "display:none;")">
@if (Metadata != null)
{
<div class="verification-layout">
@@ -70,8 +77,8 @@
<div class="actions">
<NexusButton Class="btn-secondary" OnClick="Reset" Disabled="IsIngesting">Back</NexusButton>
<NexusButton Class="@($"btn-primary {(IsIngesting ? "btn-loading" : "")}")"
mjasin marked this conversation as resolved
Antigravity commented 2026-06-01 16:31:29 +00:00
Review
Copy Link
Copy Source

🟢 Button spinner runs inside a display:none parent — unnecessary GPU compositor work

The NexusButton with btn-loading class is inside .verification-state, which is hidden via display:none when IsIngesting is true. The CSS spinner animation (spin 0.8s linear infinite) and the new filter: drop-shadow(...) are applied to DOM nodes that are not rendered to the screen.

While browsers generally optimize display:none subtrees, filter properties can still hold GPU resources depending on the browser's paint/compositor implementation. Since ingesting-state now provides its own dedicated spinner, consider simplifying the button:

<NexusButton Class="btn-primary" OnClick="SaveToLibrary" Disabled="IsIngesting">
    Save to Library
</NexusButton>

The btn-loading state on the button is redundant now that ingesting-state gives full visual feedback.

🟢 **Button spinner runs inside a `display:none` parent — unnecessary GPU compositor work** The `NexusButton` with `btn-loading` class is inside `.verification-state`, which is hidden via `display:none` when `IsIngesting` is true. The CSS spinner animation (`spin 0.8s linear infinite`) and the new `filter: drop-shadow(...)` are applied to DOM nodes that are not rendered to the screen. While browsers generally optimize `display:none` subtrees, `filter` properties can still hold GPU resources depending on the browser's paint/compositor implementation. Since `ingesting-state` now provides its own dedicated spinner, consider simplifying the button: ```razor <NexusButton Class="btn-primary" OnClick="SaveToLibrary" Disabled="IsIngesting"> Save to Library </NexusButton> ``` The `btn-loading` state on the button is redundant now that `ingesting-state` gives full visual feedback.
OnClick="SaveToLibrary"
Disabled="IsIngesting">
OnClick="SaveToLibrary"
Disabled="IsIngesting">
@(IsIngesting ? "" : "Save to Library")
</NexusButton>
</div>
@@ -79,7 +86,7 @@
</div>
<div class="upload-state @(_isDragging ? "drag-over" : "")"
style="@(!IsParsing && !IsVerifying && !IsIndexing ? "display:flex;" : "display:none;")"
style="@(!IsParsing && !IsVerifying && !IsIndexing && !IsIngesting ? "display:flex;" : "display:none;")"
@ondragenter="OnDragEnter"
@ondragleave="OnDragLeave">
<div class="drop-zone">
src/NexusReader.UI.Shared/Components/Organisms/BookIngestionModal.razor.css
+8 -5
View File
@@ -118,8 +118,9 @@
z-index: 10;
}
/* Parsing State */
.parsing-state {
/* Parsing and Ingesting States */
.parsing-state,
.ingesting-state {
flex: 1;
display: flex;
justify-content: center;
@@ -158,7 +159,8 @@
filter: drop-shadow(0 0 8px rgba(0, 255, 153, 0.3));
}
.parsing-state p {
.parsing-state p,
.ingesting-state p {
color: var(--nexus-text);
font-family: var(--nexus-font-mono, monospace);
font-size: 0.9rem;
@@ -371,10 +373,11 @@
position: absolute;
width: 20px;
height: 20px;
border: 2px solid rgba(0, 0, 0, 0.1);
border-top-color: #000;
border: 2px solid rgba(255, 255, 255, 0.2);
border-top-color: var(--nexus-neon, #00ffaa);
border-radius: 50%;
animation: spin 0.8s linear infinite;
filter: drop-shadow(0 0 4px var(--nexus-neon, #00ffaa));
}
/* Indexing State */