fix(book): loan calendar uses flatpickr on mobile (not the native picker) — shows availability#198
fix(book): loan calendar uses flatpickr on mobile (not the native picker) — shows availability#198fabiodalez-dev wants to merge 1 commit into
Conversation
… picker On the public book page the loan/reservation date picker (a flatpickr inside the SweetAlert) fell back to the native Android/iOS date picker on phones — and the native picker ignores flatpickr's `disable` option, so fully-booked days looked selectable and availability was never shown. Set disableMobile:true on the shared baseOpts so both the start and end pickers always use flatpickr's own calendar, where unavailable dates are disabled and styled. Adds tests/issue-mobile-datepicker.spec.js (Pixel 5 / Android-Chrome emulation): opens the request-loan dialog and asserts flatpickr is used (disableMobile true, isMobile false, no native input type=date, flatpickr calendar in the DOM).
📝 WalkthroughWalkthroughViene forzato l’uso di flatpickr anche su mobile nel flusso di prenotazione/prestito. È stata aggiunta una regressione Playwright che verifica il calendario Flatpickr su dispositivo mobile e l’assenza del picker nativo. ChangesComportamento del date picker mobile
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/issue-mobile-datepicker.spec.js`:
- Around line 63-71: The mobile datepicker spec is too permissive because the
login redirect is swallowed and the loan-request button check can silently skip
the test, creating a false green. In tests/issue-mobile-datepicker.spec.js,
update the login flow around page.waitForURL and the `#btn-request-loan` lookup so
failures surface explicitly instead of being ignored or skipped; keep the test
in the executed path and fail when the expected redirect or button is missing,
using the existing page.goto, page.waitForURL, and btn locator flow to locate
the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 53289992-796a-4ae4-99c0-2d2e377dafbd
📒 Files selected for processing (2)
app/Views/frontend/book-detail.phptests/issue-mobile-datepicker.spec.js
| await page.goto(`${BASE}/accedi`); | ||
| await page.fill('input[name="email"]', ADMIN_EMAIL); | ||
| await page.fill('input[name="password"]', ADMIN_PASS); | ||
| await page.locator('button[type="submit"]').click(); | ||
| await page.waitForURL(/(\/$|admin|profilo|account)/, { timeout: 15000 }).catch(() => {}); | ||
|
|
||
| await page.goto(`${BASE}/libro/${bookId}`); | ||
| const btn = page.locator('#btn-request-loan'); | ||
| test.skip((await btn.count()) === 0, 'This book has no request-loan button'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Non rendere questo spec “false green”.
Il catch(() => {}) su waitForURL e lo test.skip sul bottone permettono allo spec di passare senza verificare nulla se il login rompe, cambia il redirect o #btn-request-loan non compare. Qui dovrebbe fallire esplicitamente, altrimenti la regressione mobile resta scoperta.
Diff proposta
await page.goto(`${BASE}/accedi`);
await page.fill('input[name="email"]', ADMIN_EMAIL);
await page.fill('input[name="password"]', ADMIN_PASS);
await page.locator('button[type="submit"]').click();
- await page.waitForURL(/(\/$|admin|profilo|account)/, { timeout: 15000 }).catch(() => {});
+ await page.waitForURL(/(\/$|admin|profilo|account)/, { timeout: 15000 });
await page.goto(`${BASE}/libro/${bookId}`);
const btn = page.locator('`#btn-request-loan`');
- test.skip((await btn.count()) === 0, 'This book has no request-loan button');
+ await expect(btn).toBeVisible();Based on learnings, negli E2E di questo repo il gating deve evitare skip silenziosi che nascondono il fatto che il flusso non è stato davvero eseguito.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await page.goto(`${BASE}/accedi`); | |
| await page.fill('input[name="email"]', ADMIN_EMAIL); | |
| await page.fill('input[name="password"]', ADMIN_PASS); | |
| await page.locator('button[type="submit"]').click(); | |
| await page.waitForURL(/(\/$|admin|profilo|account)/, { timeout: 15000 }).catch(() => {}); | |
| await page.goto(`${BASE}/libro/${bookId}`); | |
| const btn = page.locator('#btn-request-loan'); | |
| test.skip((await btn.count()) === 0, 'This book has no request-loan button'); | |
| await page.goto(`${BASE}/accedi`); | |
| await page.fill('input[name="email"]', ADMIN_EMAIL); | |
| await page.fill('input[name="password"]', ADMIN_PASS); | |
| await page.locator('button[type="submit"]').click(); | |
| await page.waitForURL(/(\/$|admin|profilo|account)/, { timeout: 15000 }); | |
| await page.goto(`${BASE}/libro/${bookId}`); | |
| const btn = page.locator('`#btn-request-loan`'); | |
| await expect(btn).toBeVisible(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/issue-mobile-datepicker.spec.js` around lines 63 - 71, The mobile
datepicker spec is too permissive because the login redirect is swallowed and
the loan-request button check can silently skip the test, creating a false
green. In tests/issue-mobile-datepicker.spec.js, update the login flow around
page.waitForURL and the `#btn-request-loan` lookup so failures surface explicitly
instead of being ignored or skipped; keep the test in the executed path and fail
when the expected redirect or button is missing, using the existing page.goto,
page.waitForURL, and btn locator flow to locate the fix.
Source: Learnings
|
@coderabbitai review |
✅ Action performedReview finished.
|
Code reviewBranch: Automated 6-lens review (correctness, structure/blast-radius, CLAUDE.md compliance, comments, UX, security) over the diff. Verdict: nothing blocking. The fix ( Minor note (below threshold, non-blocking)
Lenses: L1/L2/L3/L4/L6 no findings; L5 (UX) one minor finding, classified below the validation gate. |
fabiodalez-dev
changed the title
1 participant
On the single-book page, on mobile, the loan/reservation calendar (flatpickr inside the SweetAlert) fell back to the native Android/iOS date picker. The native picker ignores flatpickr's
disableoption → fully-booked days looked selectable and availability was not shown.Fix
Added
disableMobile: trueto the sharedbaseOptsof the two pickers (start/end) inbook-detail.php: flatpickr always uses its own calendar, where unavailable dates are disabled and colored — on mobile too.Test
tests/issue-mobile-datepicker.spec.js(Pixel 5 / Android Chrome emulation): opens the loan dialog and asserts flatpickr is used —disableMobile=true,isMobile=false, no nativeinput[type=date], flatpickr calendar present in the DOM. ✅ green.PHPStan L5 green.
Summary by CodeRabbit
Bug Fixes
Tests