fix: Use inline themed SVG for loader spinner#2379
Conversation
Import tail-spin.svg as raw SVG markup and render it through the loader DOM instead of using the circular border/background spinner. Update the SVG to inherit currentColor, expand its viewBox to avoid edge clipping, and style the dialog spinner with the app primary theme color. Add ?raw SVG handling to both Rspack and Webpack so inline SVG imports do not conflict with normal asset/resource SVG usage. Also removes the old theme-side tail-spin recoloring path now that the spinner color is driven by CSS.
Greptile SummaryThis PR replaces the old background-image spinner (a pre-recolored file written to
Confidence Score: 5/5The change is safe to merge — it replaces a fragile file-system recoloring path with a straightforward inline SVG approach driven entirely by CSS, with no user-controlled data flowing into the new innerHTML assignments. The core logic — unique gradient ID stamping via createTailSpinSvg(), singleton title loader DOM reuse, and mutually exclusive ?raw/asset webpack rules — is all implemented correctly. The removal of the DATA_STORAGE write path in list.js simplifies the theme application cycle without dropping any required behavior. No correctness issues were found in the changed code. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller as showFileInfo / other
participant Loader as loader.js
participant DOM as DOM / app
participant CSS as CSS (main.scss)
Caller->>Loader: showTitleLoader()
Loader->>Loader: "createTitleLoader()<br/>get or create #__title-loader span<br/>with uniquified SVG gradient ID"
Loader->>DOM: "app.append(#__title-loader) if not connected"
Loader->>DOM: "classList.remove(title-loading-hide)<br/>classList.add(title-loading)"
DOM->>CSS: ".title-loading #__title-loader display:flex, appear animation"
Caller->>Loader: removeTitleLoader()
Loader->>DOM: classList.add(title-loading-hide)
DOM->>CSS: ".title-loading.title-loading-hide #__title-loader opacity:0, hide animation"
Note over DOM,CSS: #__title-loader stays in DOM,<br/>pointer-events:none prevents interaction
Caller->>Loader: loader.create(title, message)
Loader->>Loader: "createTailSpinSvg()<br/>increment tailSpinSvgId unique gradient ID"
Loader->>DOM: "append #__loader dialog with inlined SVG"
Note over Loader,DOM: SVG color driven by<br/>CSS color:var(--primary-color)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Caller as showFileInfo / other
participant Loader as loader.js
participant DOM as DOM / app
participant CSS as CSS (main.scss)
Caller->>Loader: showTitleLoader()
Loader->>Loader: "createTitleLoader()<br/>get or create #__title-loader span<br/>with uniquified SVG gradient ID"
Loader->>DOM: "app.append(#__title-loader) if not connected"
Loader->>DOM: "classList.remove(title-loading-hide)<br/>classList.add(title-loading)"
DOM->>CSS: ".title-loading #__title-loader display:flex, appear animation"
Caller->>Loader: removeTitleLoader()
Loader->>DOM: classList.add(title-loading-hide)
DOM->>CSS: ".title-loading.title-loading-hide #__title-loader opacity:0, hide animation"
Note over DOM,CSS: #__title-loader stays in DOM,<br/>pointer-events:none prevents interaction
Caller->>Loader: loader.create(title, message)
Loader->>Loader: "createTailSpinSvg()<br/>increment tailSpinSvgId unique gradient ID"
Loader->>DOM: "append #__loader dialog with inlined SVG"
Note over Loader,DOM: SVG color driven by<br/>CSS color:var(--primary-color)
Reviews (2): Last reviewed commit: "fix: duplicate gradient id in spinner.sv..." | Re-trigger Greptile |
| <linearGradient x1="8.042%" y1="0%" x2="65.682%" y2="23.865%" id="tail-spin-gradient"> | ||
| <stop stop-color="currentColor" stop-opacity="0" offset="0%"/> | ||
| <stop stop-color="currentColor" stop-opacity=".631" offset="63.146%"/> | ||
| <stop stop-color="currentColor" offset="100%"/> | ||
| </linearGradient> |
There was a problem hiding this comment.
Duplicate gradient ID when both spinners are shown simultaneously
The linearGradient carries id="tail-spin-gradient". When the title loader and the dialog loader are both visible (e.g., showTitleLoader is called while a dialog loader is already open), two copies of this SVG are inlined in the document, producing two elements with the same ID. Browsers resolve stroke="url(#tail-spin-gradient)" against the first matching element in the document, so the second spinner's path references the first SVG's gradient. While the gradients are identical and both spinners render correctly in practice, the duplicate ID is technically invalid and can generate console warnings in strict parsers. Scoping the ID (e.g., a unique suffix per instance) or using an inline gradientUnits="userSpaceOnUse" approach without an ID reference would eliminate the ambiguity.
|
@greptile_apps review again |
Import tail-spin.svg as raw SVG markup and render it through the loader DOM instead of using the circular border/background spinner. Update the SVG to inherit currentColor, expand its viewBox to avoid edge clipping, and style the dialog spinner with the app primary theme color.
Add ?raw SVG handling to both Rspack and Webpack so inline SVG imports do not conflict with normal asset/resource SVG usage. Also removes the old theme-side tail-spin recoloring path now that the spinner color is driven by CSS.