🎨 Palette: Add loading spinner and preserve SVG icon in share button - #144
🎨 Palette: Add loading spinner and preserve SVG icon in share button#144alvin000009238 wants to merge 1 commit into
Conversation
Adds a spinning animation icon to buttons like "Create Share Link" during async operations. Replaces the `textContent` resetting with `innerHTML` injection to prevent the button's internal SVG icon from disappearing, solving a visual layout shift when users clicked the button. Co-authored-by: alvin000009238 <107313913+alvin000009238@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request enhances the user interface by adding a loading spinner and icons to the share link creation button, along with the required CSS animations and updated build assets. The review feedback suggests improving accessibility by adding aria-hidden to the decorative SVG and refactoring the button state restoration to avoid hardcoding the HTML content twice.
| createLinkBtn.disabled = true; | ||
| createLinkBtn.textContent = '建立中...'; | ||
| createLinkBtn.innerHTML = ` | ||
| <svg class="spinner-icon" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"> | ||
| <path d="M21 12a9 9 0 1 1-6.219-8.56"></path> | ||
| </svg> | ||
| 建立中... | ||
| `; |
There was a problem hiding this comment.
Capture the original button content and add aria-hidden="true" to the spinner SVG. Storing the initial state allows for a cleaner restoration in the error handler without duplicating the SVG markup, while aria-hidden ensures the decorative icon is ignored by screen readers.
| createLinkBtn.disabled = true; | |
| createLinkBtn.textContent = '建立中...'; | |
| createLinkBtn.innerHTML = ` | |
| <svg class="spinner-icon" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"> | |
| <path d="M21 12a9 9 0 1 1-6.219-8.56"></path> | |
| </svg> | |
| 建立中... | |
| `; | |
| const originalBtnHtml = createLinkBtn.innerHTML; | |
| createLinkBtn.disabled = true; | |
| createLinkBtn.innerHTML = ` | |
| <svg class="spinner-icon" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"> | |
| <path d="M21 12a9 9 0 1 1-6.219-8.56"></path> | |
| </svg> | |
| 建立中... | |
| `; |
| createLinkBtn.innerHTML = ` | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"> | ||
| <path d="M10 13a5 5 0 0 0 7.54.54l3-3a5 5 0 0 0-7.07-7.07l-1.72 1.71"></path> | ||
| <path d="M14 11a5 5 0 0 0-7.54-.54l-3 3a5 5 0 0 0 7.07 7.07l1.71-1.71"></path> | ||
| </svg> | ||
| 建立分享連結 | ||
| `; |
There was a problem hiding this comment.
Pull request overview
Adds a loading spinner state to the “Create Share Link” button to avoid losing the button’s SVG icon during async share-link creation, and updates the built asset hashes referenced by the static HTML entrypoint.
Changes:
- Swap the share button’s content to a spinner+text loading state during link creation, and restore the original icon on failure.
- Add a
.spinner-iconCSS animation. - Update
public/index.htmlreferences to the latest hashed/dist/CSS/JS bundles.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| public/index.html | Updates hashed /dist/ asset filenames referenced by the production HTML. |
| frontend/styles/motion.css | Adds spinner keyframes + .spinner-icon animation styling. |
| frontend/share.js | Implements loading-state spinner markup for the share-link creation button and restores original icon markup on error. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @keyframes spin { | ||
|
|
||
| from { | ||
|
|
||
| transform: rotate(0deg); | ||
|
|
||
| } | ||
|
|
||
| to { | ||
|
|
||
| transform: rotate(360deg); | ||
|
|
||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
@keyframes spin is already defined elsewhere (e.g., in frontend/styles/dashboard.css) and this new definition in motion.css will override it because motion.css is imported later in frontend/style.css. That can unintentionally change other animations that rely on spin (like the dashboard avatar ring). Consider renaming this new keyframes to a more specific name (e.g., spinner-spin) and updating .spinner-icon to use that name, or reusing the existing spin definition without redefining it.
|
|
||
| animation: spin 1s linear infinite; | ||
|
|
||
| } |
There was a problem hiding this comment.
This introduces an infinite rotation animation for .spinner-icon, but there’s no prefers-reduced-motion handling. Please add a reduced-motion fallback (e.g., disable the animation under @media (prefers-reduced-motion: reduce)) so users who request reduced motion aren’t forced to see a continuously spinning indicator.
| } | |
| } | |
| @media (prefers-reduced-motion: reduce) { | |
| .spinner-icon { | |
| animation: none; | |
| } | |
| } |
| createLinkBtn.disabled = true; | ||
| createLinkBtn.textContent = '建立中...'; | ||
| createLinkBtn.innerHTML = ` | ||
| <svg class="spinner-icon" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"> | ||
| <path d="M21 12a9 9 0 1 1-6.219-8.56"></path> | ||
| </svg> | ||
| 建立中... | ||
| `; |
There was a problem hiding this comment.
The loading-state UI is being built via innerHTML. Even though the string is currently static, this duplicates button markup and makes future icon/text updates easy to miss (you now have to keep multiple SVG snippets in sync). Consider capturing the original button contents once (e.g., before the first swap) and restoring from that, or toggling a CSS class / swapping only the icon node instead of rewriting HTML.
| createLinkBtn.innerHTML = ` | ||
| <svg class="spinner-icon" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"> | ||
| <path d="M21 12a9 9 0 1 1-6.219-8.56"></path> | ||
| </svg> |
There was a problem hiding this comment.
The spinner SVG is decorative and sits next to button text; without aria-hidden (and related SVG accessibility attributes), some screen readers may announce it as an extra graphic. Consider marking the spinner icon as decorative (e.g., aria-hidden="true", and focusable="false" for broader SVG compatibility) while keeping the text as the accessible label.
| createLinkBtn.disabled = true; | ||
| createLinkBtn.textContent = '建立中...'; | ||
| createLinkBtn.innerHTML = ` | ||
| <svg class="spinner-icon" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"> | ||
| <path d="M21 12a9 9 0 1 1-6.219-8.56"></path> | ||
| </svg> | ||
| 建立中... | ||
| `; |
There was a problem hiding this comment.
There are existing tests/frontend/share.test.js tests for share.js, but the updated setupShareFeature() behavior (loading state disables the button, swaps to spinner, and restores the original icon/text on failure) isn’t covered. Adding a small jsdom-based test around the click flow would help prevent regressions in the button’s icon restoration and loading UI.
💡 What: Added a spinning animation to the "Create Share Link" button during its loading state and ensured the original icon returns.
🎯 Why: Changing
textContentdynamically wiped out the SVG icon inside the button, causing visual layout shifts and a loss of visual context. Adding a spinner provides immediate feedback.📸 Before/After: Before, clicking "Create Link" resulted in a text-only "建立中..." that shrunk the button and dropped the link icon. Now, it swaps the link icon for a rotating spinner, keeping the same structure, and restores the original icon properly.
♿ Accessibility: Provides clearer visual feedback during asynchronous network requests.
PR created automatically by Jules for task 9858335729685408024 started by @alvin000009238