test vibrate 1#34
Conversation
📝 WalkthroughWalkthroughAdded a local vibrarTelefono(pattern) helper and short vibration calls in App.tsx and two hooks; some vibration invocations in the audio controller remain commented out. No exported APIs were changed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Hook as useAudioController / useCampanadasTimer
participant Browser as Navigator.Vibrate
User->>App: triggers action (click / start / message)
App->>Hook: (may) call vibrarTelefono(pattern)
Hook->>Browser: feature-detect navigator.vibrate
alt supported
Hook->>Browser: navigator.vibrate(pattern)
Browser-->>Hook: success/fulfilled
else unsupported or blocked
Browser-->>Hook: undefined / rejected
Hook-->>App: console log / handle no-vibration
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Deploying campanadas with
|
| Latest commit: |
e103563
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://30c97632.campanadas.pages.dev |
| Branch Preview URL: | https://test-vibrate.campanadas.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/useAudioController.ts (1)
64-72: Guardnavigator.vibratefor cross-browser compatibility (missing in iOS Safari and desktop Safari).The call at line 70 is unguarded. While iOS Safari silently ignores the API rather than throwing, proper feature-detection is needed to ensure the code works safely across browsers (Chromium, Firefox, and Safari variants have inconsistent support).
Use the standard feature-detect pattern per MDN:
Proposed fix
if (currentChimeIndex !== prevChimeIndexRef.current && currentChimeIndex < 12) { audioService.playChime(); - navigator.vibrate([200, 200]); + if ('vibrate' in navigator) { + navigator.vibrate([200, 200]); + } prevChimeIndexRef.current = currentChimeIndex; }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @hooks/useCampanadasTimer.ts:
- Around line 28-39: Extract the duplicated vibrarTelefono implementation into a
single shared utility module (e.g., export function vibrarTelefono from a new
vibration util) and add TypeScript types: declare type VibrationPattern = number
| number[] and export const vibrarTelefono = (pattern: VibrationPattern): void
=> { ... }; then replace the local implementations in App.tsx,
useCampanadasTimer.ts and useAudioController.ts to import and call the shared
vibrarTelefono, keeping the existing console/log behavior and runtime checks for
"vibrate" in navigator.
- Line 43: The call to vibrarTelefono([300, 300]) inside the useEffect
initialization of useCampanadasTimer will be blocked by browsers because
vibration must be triggered by a user gesture; remove that call from the
useEffect and instead invoke vibrarTelefono from a user-initiated handler (e.g.,
a click/tap or explicit "play"/"ring" action) tied to the component that uses
useCampanadasTimer; update any callers or event handlers to call vibrarTelefono
(or expose a startVibration function from the hook) and keep the defensive
logging in place if the API still returns false.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
App.tsxhooks/useAudioController.tshooks/useCampanadasTimer.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/useAudioController.ts
🧰 Additional context used
🧬 Code graph analysis (1)
App.tsx (1)
config/constants.ts (2)
TARGET_DATE(2-2)INSTRUCTIONS_MODAL_AUTO_CLOSE_MS(19-19)
🔇 Additional comments (1)
App.tsx (1)
62-62: LGTM—vibration calls are properly triggered by user gestures.All four vibration call sites (grape click, test start, early-click message, instructions message) are correctly tied to user interactions, ensuring they comply with browser security policies for the Vibration API.
Also applies to: 101-101, 141-141, 149-149
| const vibrarTelefono = (pattern) => { | ||
| console.log("Vibrando", pattern); | ||
| if ("vibrate" in navigator) { | ||
| // Intentar vibrar 500ms | ||
| const exito = navigator.vibrate(pattern); | ||
| if (!exito) { | ||
| console.log("La vibración fue bloqueada o no es compatible."); | ||
| } | ||
| } else { | ||
| console.log("Tu navegador no soporta la API de vibración."); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated function to a shared utility module and add TypeScript types.
This function is duplicated across at least three files (App.tsx, useCampanadasTimer.ts, and useAudioController.ts per the AI summary). Consider extracting it to a shared utility module (e.g., utils/vibration.ts) with proper TypeScript typing:
type VibrationPattern = number | number[];
export const vibrarTelefono = (pattern: VibrationPattern): void => {
console.log("Vibrando", pattern);
if ("vibrate" in navigator) {
const exito = navigator.vibrate(pattern);
if (!exito) {
console.log("La vibración fue bloqueada o no es compatible.");
}
} else {
console.log("Tu navegador no soporta la API de vibración.");
}
};Then import and use it across all three files.
🤖 Prompt for AI Agents
In @hooks/useCampanadasTimer.ts around lines 28 - 39, Extract the duplicated
vibrarTelefono implementation into a single shared utility module (e.g., export
function vibrarTelefono from a new vibration util) and add TypeScript types:
declare type VibrationPattern = number | number[] and export const
vibrarTelefono = (pattern: VibrationPattern): void => { ... }; then replace the
local implementations in App.tsx, useCampanadasTimer.ts and
useAudioController.ts to import and call the shared vibrarTelefono, keeping the
existing console/log behavior and runtime checks for "vibrate" in navigator.
| useEffect(() => { | ||
| let animationFrameId: number; | ||
|
|
||
| vibrarTelefono([300, 300]); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Web Vibration API user gesture requirement browser security policy
💡 Result:
Short answer
- The Vibration API (navigator.vibrate()) is gated by user activation: browsers require a user gesture (sticky activation — the page must have been interacted with) for vibration to run; otherwise the call is blocked or returns false. [1][2][3][4]
Key points / security rationale
- This is a deliberate policy to prevent abusive/background vibration (and to treat first‑ and third‑party contexts separately). Browsers may also choose to further restrict/disable vibration (e.g., in cross‑origin iframes). [3][4][1]
Browser behaviour notes
- Chromium-family: shipped a user‑gesture requirement (calls return false when no user activation); there is source/flag text and related issues. Cross‑origin iframe uses were also limited. [2][5]
- Firefox/Opera/Samsung have implemented similar gating and iframe restrictions; Safari historically lacks support. Check current browser versions for exact behavior. [1][3]
References
- MDN — Navigator.vibrate() (security: sticky user activation required). [MDN]. [link].
- Chromium source & feature discussion: "Requiring user gesture for the Vibration API" (flag/issue). [chromium]. [link].
- HTML Standard — user activation / activation‑gated APIs (sticky activation semantics). [WHATWG HTML]. [link].
- W3C/public-device-apis log and Vibration API discussion describing the spec change to require user activation. [W3C]. [link].
If you want, I can fetch exact browser-version behavior (which versions enforce the rule, iframe specifics, and example detection code).
🏁 Script executed:
# Find and read the useCampanadasTimer.ts file to understand context
fd -t f "useCampanadasTimer.ts" --exec cat -n {}Repository: emaza/campanadas
Length of output: 3688
Move vibration calls outside of useEffect initialization to user-initiated events.
The Vibration API requires user activation (a sticky gesture like click or tap) to function in modern browsers (Chrome, Firefox, Opera, Samsung). Calling vibrarTelefono([300, 300]) on line 43 during useEffect initialization runs before any user interaction and will be silently blocked—the navigator.vibrate() call will return false in most browsers.
The current defensive logging catches this silently, but the vibration won't work. Move vibration triggers to event handlers or other user-initiated actions instead.
🤖 Prompt for AI Agents
In @hooks/useCampanadasTimer.ts at line 43, The call to vibrarTelefono([300,
300]) inside the useEffect initialization of useCampanadasTimer will be blocked
by browsers because vibration must be triggered by a user gesture; remove that
call from the useEffect and instead invoke vibrarTelefono from a user-initiated
handler (e.g., a click/tap or explicit "play"/"ring" action) tied to the
component that uses useCampanadasTimer; update any callers or event handlers to
call vibrarTelefono (or expose a startVibration function from the hook) and keep
the defensive logging in place if the API still returns false.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hooks/useCampanadasTimer.ts (1)
41-47: Don’t leave commented-out vibration calls inuseEffect; if enabled later, ensure it can’t run in the RAF loop.
If vibration is intended, wire it to a phase transition (or explicit user gesture) rather than anything that could accidentally run per-frame.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
hooks/useCampanadasTimer.ts
🔇 Additional comments (1)
hooks/useCampanadasTimer.ts (1)
83-87: Whitespace-only change looks fine.
No behavior change spotted here.
| const vibrarTelefono = (pattern) => { | ||
| console.log("Vibrando", pattern); | ||
| if ("vibrate" in navigator) { | ||
| // Intentar vibrar 500ms | ||
| const exito = navigator.vibrate(pattern); | ||
| if (!exito) { | ||
| console.log("La vibración fue bloqueada o no es compatible."); | ||
| } | ||
| } else { | ||
| console.log("Tu navegador no soporta la API de vibración."); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file
find . -type f -name "useCampanadasTimer.ts" | head -5Repository: emaza/campanadas
Length of output: 89
🏁 Script executed:
# Let's also search for VibratePattern type in the codebase
rg "VibratePattern" --type tsRepository: emaza/campanadas
Length of output: 42
🏁 Script executed:
# Check how vibrarTelefono is used/called in the file
rg "vibrarTelefono" --type ts -B 2 -A 2Repository: emaza/campanadas
Length of output: 2646
🌐 Web query:
navigator.vibrate() API requirements user-gesture HTTPS browser support MDN
💡 Result:
Summary — navigator.vibrate() (MDN)
- Secure context: available only in secure contexts (HTTPS). [1]
- User gesture: requires a sticky user activation (user interaction) for the call to actually vibrate. [1]
- Browser support (high level): supported in Chromium-based browsers, Firefox and many Android browsers; not supported (or very limited) in Safari / iOS Safari. Some engines disable vibration in cross-origin iframes. Implementation history: Chromium (Chrome/Android) began requiring a user gesture around Chrome 60 and disallowed cross-origin iframe vibration around Chrome 55; Opera and Samsung browsers have similar restrictions; Firefox behavior differs slightly across versions (pattern truncation, iframe rules). See browser-compat tables for exact version details. [1][2]
Sources
- Navigator.vibrate() — MDN Web Docs. [1]
- Vibration API — MDN Web Docs (browser-compat/details and implementation notes). [2]
🏁 Script executed:
# Get the full context of useCampanadasTimer.ts
wc -l ./hooks/useCampanadasTimer.tsRepository: emaza/campanadas
Length of output: 92
🏁 Script executed:
# Read the file to see the complete implementation
cat -n ./hooks/useCampanadasTimer.ts | head -50Repository: emaza/campanadas
Length of output: 1810
🏁 Script executed:
# Check if there are any lint configs or console rules
find . -type f -name ".eslintrc*" -o -name "eslint.config.*" -o -name "lint.config.*" | head -5Repository: emaza/campanadas
Length of output: 42
🏁 Script executed:
# Check if console is being used elsewhere and how it's handled
rg "console\.(log|warn|error)" --type ts --stats | head -20Repository: emaza/campanadas
Length of output: 1013
Extract to a shared utility, add proper types and browser/environment guards, and remove console logs.
This function is duplicated identically in three files (App.tsx, useCampanadasTimer.ts, useAudioController.ts) and should be consolidated into a shared hook or utility.
Additionally, the pattern parameter is untyped, and the direct navigator reference will crash in test or non-browser environments without proper guards. Console logs should be removed from production hooks.
Suggested improvements
Create a shared hook (e.g., hooks/useVibration.ts):
+export const useVibration = () => {
+ const vibrate = (pattern: number | number[] = [200, 200]): boolean => {
+ if (typeof navigator === 'undefined' || typeof navigator.vibrate !== 'function') {
+ return false;
+ }
+ return navigator.vibrate(pattern);
+ };
+
+ return { vibrate };
+};Then import and use in all three files:
- const vibrarTelefono = (pattern) => {
- console.log("Vibrando", pattern);
- if ("vibrate" in navigator) {
- const exito = navigator.vibrate(pattern);
- if (!exito) {
- console.log("La vibración fue bloqueada o no es compatible.");
- }
- } else {
- console.log("Tu navegador no soporta la API de vibración.");
- }
- };
+ const { vibrate } = useVibration();Note: navigator.vibrate() requires HTTPS and an active user gesture (which your current usage on user interactions already satisfies), and has limited support on Safari/iOS.
Committable suggestion skipped: line range outside the PR's diff.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.