feat: add pulse indicator when has unsaved changes#87
Open
joaovictorjs wants to merge 2 commits intowebadderall:mainfrom
Open
feat: add pulse indicator when has unsaved changes#87joaovictorjs wants to merge 2 commits intowebadderall:mainfrom
joaovictorjs wants to merge 2 commits intowebadderall:mainfrom
Conversation
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
UI Visual Indicator src/components/video-editor/VideoEditor.tsx |
Added conditional pinging sky‑blue dot inside Save button shown when hasUnsavedChanges === true; minor JSX indentation adjustment for Export dropdown trigger Button (no behavior change). |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
🐰 A tiny blue ping in sight,
Hops to warn of changes light,
Tap save quick, keep work in tune,
Moonlit edits, hummed by June.
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | ❓ Inconclusive | The description is minimal but covers the core purpose. However, it lacks several required template sections including Motivation, Type of Change, Testing Guide, and Checklist. | Enhance the description by adding missing sections from the template: explain the motivation/problem solved, specify the type of change, provide testing steps, and complete the checklist items. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately describes the main change: adding a visual pulse indicator to the Save button that appears when there are unsaved changes. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/video-editor/VideoEditor.tsx (1)
2380-2383: Add accessibility support for the unsaved changes indicator.The visual indicator is effective but screen reader users won't be informed of unsaved changes. Consider adding an accessible label to the button or a screen-reader-only text element.
Also, lines 2381-2382 have inconsistent leading whitespace (extra space before the tab character).
♻️ Suggested improvement with accessibility and whitespace fix
<Button type="button" onClick={handleSaveProject} className="inline-flex h-8 min-w-[96px] items-center justify-center gap-1.5 rounded-[5px] bg-white px-4 text-black transition-colors hover:bg-white/92" + aria-label={hasUnsavedChanges ? t("common.actions.save") + " - " + t("common.status.unsavedChanges", "unsaved changes") : t("common.actions.save")} > <span className={`${hasUnsavedChanges ? "flex" : "hidden"} size-2 relative`}> - <span className="absolute inline-flex h-full w-full animate-ping rounded-full bg-sky-400 opacity-75"></span> - <span className="relative inline-flex size-2 rounded-full bg-sky-500"></span> + <span className="absolute inline-flex h-full w-full animate-ping rounded-full bg-sky-400 opacity-75"></span> + <span className="relative inline-flex size-2 rounded-full bg-sky-500"></span> </span> <Save className="h-4 w-4" /> <span className="text-sm font-semibold tracking-tight">{t("common.actions.save")}</span> </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 2380 - 2383, The unsaved-changes indicator in VideoEditor.tsx uses only visual cues (the span controlled by hasUnsavedChanges) and lacks accessibility for screen readers; update the markup around the indicator (the span that toggles with hasUnsavedChanges) to include an accessible label — either add aria-hidden="true" to the decorative ping spans and add a sibling screen-reader-only element (e.g., visually-hidden text with role="status" or aria-live="polite") that reads "Unsaved changes" when hasUnsavedChanges is true, or set an appropriate aria-label/aria-live on the container so assistive tech is informed; also remove the extra leading whitespace on the two inner span lines to fix the formatting inconsistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2380-2383: The unsaved-changes indicator in VideoEditor.tsx uses
only visual cues (the span controlled by hasUnsavedChanges) and lacks
accessibility for screen readers; update the markup around the indicator (the
span that toggles with hasUnsavedChanges) to include an accessible label —
either add aria-hidden="true" to the decorative ping spans and add a sibling
screen-reader-only element (e.g., visually-hidden text with role="status" or
aria-live="polite") that reads "Unsaved changes" when hasUnsavedChanges is true,
or set an appropriate aria-label/aria-live on the container so assistive tech is
informed; also remove the extra leading whitespace on the two inner span lines
to fix the formatting inconsistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0d728225-e196-4600-a624-4ab9b23d5ee9
📒 Files selected for processing (1)
src/components/video-editor/VideoEditor.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/video-editor/VideoEditor.tsx (1)
2380-2383: Nice implementation of the unsaved changes indicator.The pulsing dot provides clear visual feedback to users about pending changes. The use of
animate-pingwith an absolute-positioned overlay creates a clean pulse effect.Minor nit: Lines 2381-2382 have an extra leading space character before the
<span>tags, causing inconsistent indentation.✨ Fix indentation
<span className={`${hasUnsavedChanges ? "flex" : "hidden"} size-2 relative`}> - <span className="absolute inline-flex h-full w-full animate-ping rounded-full bg-[`#2563EB`] opacity-75"></span> - <span className="relative inline-flex size-2 rounded-full bg-[`#2563EB`]"></span> + <span className="absolute inline-flex h-full w-full animate-ping rounded-full bg-[`#2563EB`] opacity-75"></span> + <span className="relative inline-flex size-2 rounded-full bg-[`#2563EB`]"></span> </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 2380 - 2383, Remove the extra leading space characters before the two inner <span> elements in the unsaved changes indicator block so their indentation matches the parent <span> and surrounding JSX; locate the snippet that uses the hasUnsavedChanges conditional and the inner spans with classes "absolute inline-flex h-full w-full animate-ping rounded-full bg-[`#2563EB`] opacity-75" and "relative inline-flex size-2 rounded-full bg-[`#2563EB`]" and adjust whitespace so both inner <span> tags start at the same indentation level as the parent for consistent formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2380-2383: Remove the extra leading space characters before the
two inner <span> elements in the unsaved changes indicator block so their
indentation matches the parent <span> and surrounding JSX; locate the snippet
that uses the hasUnsavedChanges conditional and the inner spans with classes
"absolute inline-flex h-full w-full animate-ping rounded-full bg-[`#2563EB`]
opacity-75" and "relative inline-flex size-2 rounded-full bg-[`#2563EB`]" and
adjust whitespace so both inner <span> tags start at the same indentation level
as the parent for consistent formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1fd39838-dcf6-490e-8bf8-7ffdb6d091c8
📒 Files selected for processing (1)
src/components/video-editor/VideoEditor.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a visual indicator to the "Save" button to inform the user that there are pending changes to be saved.
Preview
Summary by CodeRabbit