Bugzilla – Attachment 191353 Details for
Bug 37707
Lead/Trail times should work in combination
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37707: (follow-up) Add textual feedback for booking lead/trail period conflicts
0e07980.patch (text/plain), 38.81 KB, created by
Martin Renvoize (ashimema)
on 2026-01-13 13:31:34 UTC
(
hide
)
Description:
Bug 37707: (follow-up) Add textual feedback for booking lead/trail period conflicts
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-01-13 13:31:34 UTC
Size:
38.81 KB
patch
obsolete
>From 0e07980c884e0ec2480496250ce958df9b5a9c24 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Sat, 3 Jan 2026 07:34:09 +0000 >Subject: [PATCH] Bug 37707: (follow-up) Add textual feedback for booking > lead/trail period conflicts > >This patch adds a feedback message container below the calendar that >provides contextual information about lead/trail period requirements >and conflicts, improving accessibility and user understanding. > >The feedback updates dynamically on hover, showing: >- Info messages when hovering selectable dates >- Error messages when hovering dates disabled due to conflicts >- Specific explanations distinguishing between conflict types > (e.g., new lead vs existing trail, new trail vs existing lead) >- Multiple conflict messages when both lead and trail periods conflict >- Specific messages for past dates and insufficient lead time > >Key improvements: >- Standardized terminology consistently uses "lead period" and "trail period" >- Proper internationalization using __() for all user-facing strings >- DRY code with detectPeriodConflict() helper function >- Styling moved from inline JavaScript to SCSS using Bootstrap alert classes >- Empty/zero periods are handled gracefully (not mentioned in messages) >- Accurate conflict detection prioritizes what the hovered date IS over > what it conflicts with > >This complements the visual color indicators by providing explicit >textual explanations, improving accessibility and user understanding. > >Test plan: >1. Apply all patches and run: yarn build >2. Configure a bookable item with 2-day lead and 3-day trail periods >3. Create an existing booking for January 10-15 (protected: Jan 8-18) >4. Open the booking calendar and observe feedback area below calendar > >5. Test past date and insufficient lead time messages: > - Hover on today's date > * Verify message: "Cannot select: Date is in the past" > - Hover on tomorrow (with 2-day lead period) > * Verify message: "Cannot select: Insufficient lead time (2 days required before start)" > - Hover on the day after tomorrow > * Verify message: "Cannot select: Insufficient lead time (2 days required before start)" > * Should NOT show "conflicts with existing booking" if no booking exists > >6. Test existing booking conflict messages: > - Hover over Jan 12 (part of existing booking) > * Verify message: "Cannot select: This date is part of an existing booking" > - Hover over Jan 8 (existing booking's lead period) > * Verify message: "Cannot select: This date is part of an existing booking's lead period" > - Hover over Jan 16 (existing booking's trail period) > * Verify message: "Cannot select: This date is part of an existing booking's trail period" > >7. Test lead/trail period conflict messages: > - Try to select Jan 19-20 as booking dates (2-day lead = Jan 17-18) > - Hover over Jan 19 when it appears disabled > * Verify error message: "Cannot select: Lead period (2 days before start) conflicts with existing booking's trail period" > >8. Test info messages for valid dates: > - Hover over Jan 21 (no conflicts, with lead period configured) > * Verify info message: "Selecting start date. Lead period: 2 days before start" > - Configure booking with NO lead period (0 or blank) > - Hover over a valid date > * Verify message: "Selecting start date" (no mention of lead period) > >9. Test multiple conflict messages: > - Position calendar to hover between two bookings where both lead and trail conflict > * Verify message shows both conflicts separated by semicolon > * Example: "Cannot select: lead period conflicts with existing booking's trail period; trail period conflicts with existing booking's lead period" > >10. Verify all feedback messages: > - Are properly translated (check .po files if applicable) > - Use consistent "lead period" and "trail period" terminology > - Are clear and helpful for librarians > - Update color coding (red for errors, blue for info) >--- > .../intranet-tmpl/prog/css/src/bookings.scss | 2 +- > .../prog/css/src/staff-global.scss | 13 + > .../prog/js/modals/place_booking.js | 425 ++++++++++++++++-- > 3 files changed, 398 insertions(+), 42 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss b/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss >index 3f72fbf488f..0f9a7da6fea 100644 >--- a/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss >+++ b/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss >@@ -10,4 +10,4 @@ > ); > } > } >-} >\ No newline at end of file >+} >diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss >index a75fca56e89..8e3dec110a8 100644 >--- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss >+++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss >@@ -1103,6 +1103,19 @@ $leadClashTrail: #e5d4f5 !default; > } > } > >+// Booking conflict feedback message styles >+.booking-conflict-feedback { >+ // Override Bootstrap's default alert padding and sizing for compact display >+ padding: 0.5rem 0.75rem; >+ margin-top: 0.5rem; >+ margin-bottom: 0 !important; // Override Bootstrap's default 1rem margin-bottom >+ min-height: 1.25rem; >+ text-align: center; >+ >+ // Bootstrap's .alert-danger, .alert-warning, and .alert-info classes >+ // provide the color schemes via CSS variables when combined with .alert >+} >+ > .input-warning { > background-color: #ff9; > border-color: #900; >diff --git a/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js b/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js >index dc04dc17d49..c1dcf5ec3f8 100644 >--- a/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js >+++ b/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js >@@ -922,6 +922,17 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > ); > } > >+ // Create feedback message container below the calendar >+ let feedbackDiv = periodPicker.calendarContainer.querySelector( >+ ".booking-conflict-feedback" >+ ); >+ if (!feedbackDiv) { >+ feedbackDiv = document.createElement("div"); >+ feedbackDiv.className = >+ "booking-conflict-feedback alert d-none"; >+ periodPicker.calendarContainer.appendChild(feedbackDiv); >+ } >+ > // Add hints for days before the start range and after the end range > periodPicker.calendarContainer.addEventListener( > "mouseover", >@@ -941,9 +952,15 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > const leadStart = startDate > ? startDate.subtract(leadDays, "day") > : hoverDate.subtract(leadDays, "day"); >- const leadEnd = startDate ? startDate : hoverDate; >- const trailStart = hoverDate; >- const trailEnd = hoverDate.add(trailDays, "day"); >+ const leadEnd = startDate >+ ? startDate.subtract(1, "day") >+ : hoverDate.subtract(1, "day"); >+ const trailStart = startDate >+ ? hoverDate.add(1, "day") >+ : hoverDate.add(1, "day"); >+ const trailEnd = startDate >+ ? hoverDate.add(trailDays, "day") >+ : hoverDate.add(trailDays, "day"); > > // BIDIRECTIONAL ENHANCEMENT: Collect closest bookings for visual feedback > // and check for mathematical conflicts in a single pass >@@ -956,6 +973,18 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > let leadDisable = false; > let trailDisable = false; > >+ // Track conflict reasons for messaging >+ let leadConflictReason = { >+ withTrail: false, >+ withLead: false, >+ withBooking: false, >+ }; >+ let trailConflictReason = { >+ withTrail: false, >+ withLead: false, >+ withBooking: false, >+ }; >+ > bookings.forEach(booking => { > // Skip if we're editing this booking > if ( >@@ -1000,8 +1029,9 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > "day" > ); > >- // Check if new booking's LEAD period overlaps with existing booking's TRAIL period >+ // Check if new booking's LEAD period overlaps with existing booking > if (!periodPicker.selectedDates[0]) { >+ // Check overlap with existing booking's trail period > if ( > leadStart.isSameOrBefore( > existingTrailEnd >@@ -1011,11 +1041,31 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > ) > ) { > leadDisable = true; >+ leadConflictReason.withTrail = true; >+ } >+ // Check overlap with existing booking's lead period >+ else if ( >+ leadStart.isSameOrBefore( >+ existingLeadEnd >+ ) && >+ leadEnd.isSameOrAfter(existingLeadStart) >+ ) { >+ leadDisable = true; >+ leadConflictReason.withLead = true; >+ } >+ // Check overlap with existing booking itself >+ else if ( >+ leadStart.isSameOrBefore(bookingEnd) && >+ leadEnd.isSameOrAfter(bookingStart) >+ ) { >+ leadDisable = true; >+ leadConflictReason.withBooking = true; > } > } > >- // Check if new booking's TRAIL period overlaps with existing booking's LEAD period >+ // Check if new booking's TRAIL period overlaps with existing booking > if (periodPicker.selectedDates[0]) { >+ // Check overlap with existing booking's lead period > if ( > trailStart.isSameOrBefore( > existingLeadEnd >@@ -1025,6 +1075,27 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > ) > ) { > trailDisable = true; >+ trailConflictReason.withLead = true; >+ } >+ // Check overlap with existing booking's trail period >+ else if ( >+ trailStart.isSameOrBefore( >+ existingTrailEnd >+ ) && >+ trailEnd.isSameOrAfter( >+ existingTrailStart >+ ) >+ ) { >+ trailDisable = true; >+ trailConflictReason.withTrail = true; >+ } >+ // Check overlap with existing booking itself >+ else if ( >+ trailStart.isSameOrBefore(bookingEnd) && >+ trailEnd.isSameOrAfter(bookingStart) >+ ) { >+ trailDisable = true; >+ trailConflictReason.withBooking = true; > } > } > >@@ -1058,6 +1129,7 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > } > }); > >+ // Work through all days in view and add classes appropraitely based on hovered date > periodPicker.calendarContainer > .querySelectorAll(".flatpickr-day") > .forEach(function (dayElem) { >@@ -1085,35 +1157,46 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > "existingBookingTrailEnd" > ); > >- // Apply new booking's lead/trail period classes >- dayElem.classList.toggle( >- "leadRangeStart", >- elemDate.isSame(leadStart) >- ); >- dayElem.classList.toggle( >- "leadRange", >- elemDate.isSameOrAfter(leadStart) && >- elemDate.isBefore(leadEnd) >- ); >- dayElem.classList.toggle( >- "leadRangeEnd", >- elemDate.isSame(leadEnd) >- ); >- dayElem.classList.toggle( >- "trailRangeStart", >- elemDate.isSame(trailStart) >- ); >- dayElem.classList.toggle( >- "trailRange", >- elemDate.isAfter(trailStart) && >- elemDate.isSameOrBefore(trailEnd) >- ); >- dayElem.classList.toggle( >- "trailRangeEnd", >- elemDate.isSame(trailEnd) >- ); >+ // Apply proposed booking's lead/trail period classes >+ // Only apply lead classes if lead period > 0 >+ if (leadDays > 0) { >+ dayElem.classList.toggle( >+ "leadRangeStart", >+ elemDate.isSame(leadStart) >+ ); >+ dayElem.classList.toggle( >+ "leadRange", >+ elemDate.isSameOrAfter(leadStart) && >+ elemDate.isSameOrBefore(leadEnd) >+ ); >+ dayElem.classList.toggle( >+ "leadRangeEnd", >+ elemDate.isSame(leadEnd) >+ ); >+ } > >- // BIDIRECTIONAL: Show closest existing booking's trail period >+ // Only apply trail classes if trail period > 0 >+ if (trailDays > 0) { >+ dayElem.classList.toggle( >+ "trailRangeStart", >+ elemDate.isSame(trailStart) >+ ); >+ dayElem.classList.toggle( >+ "trailRange", >+ elemDate.isSameOrAfter( >+ trailStart >+ ) && >+ elemDate.isSameOrBefore( >+ trailEnd >+ ) >+ ); >+ dayElem.classList.toggle( >+ "trailRangeEnd", >+ elemDate.isSame(trailEnd) >+ ); >+ } >+ >+ // Show closest preceding existing booking's trail period > if (closestBeforeBooking && trailDays > 0) { > const existingTrailStart = > closestBeforeBooking.end.add( >@@ -1159,7 +1242,7 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > } > } > >- // BIDIRECTIONAL: Show closest existing booking's lead period >+ // Show closest following existing booking's lead period > if (closestAfterBooking && leadDays > 0) { > const existingLeadStart = > closestAfterBooking.start.subtract( >@@ -1203,7 +1286,7 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > } > } > >- // Check for conflicts with flatpickr-disabled dates (original behavior) >+ // Check for conflicts with flatpickr-disabled dates > if ( > dayElem.classList.contains( > "flatpickr-disabled" >@@ -1212,12 +1295,15 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > if ( > !periodPicker.selectedDates[0] && > elemDate.isSameOrAfter(leadStart) && >- elemDate.isBefore(leadEnd) >+ elemDate.isSameOrBefore(leadEnd) > ) { > leadDisable = true; > } > if ( >- elemDate.isAfter(trailStart) && >+ periodPicker.selectedDates[0] && >+ elemDate.isSameOrAfter( >+ trailStart >+ ) && > elemDate.isSameOrBefore(trailEnd) > ) { > // Only consider this a conflict if the disabled date is within the max date range >@@ -1238,7 +1324,7 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > } > } > >- // BIDIRECTIONAL: Check for conflicts with existing booking's trail period >+ // Check for conflicts with existing booking's trail period > if ( > !periodPicker.selectedDates[0] && > dayElem.classList.contains( >@@ -1248,21 +1334,24 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > // New booking's lead period overlaps with existing booking's trail > if ( > elemDate.isSameOrAfter(leadStart) && >- elemDate.isBefore(leadEnd) >+ elemDate.isSameOrBefore(leadEnd) > ) { > leadDisable = true; > } > } > >- // BIDIRECTIONAL: Check for conflicts with existing booking's lead period >+ // Check for conflicts with existing booking's lead period > if ( >+ periodPicker.selectedDates[0] && > dayElem.classList.contains( > "existingBookingLead" > ) > ) { > // New booking's trail period overlaps with existing booking's lead > if ( >- elemDate.isAfter(trailStart) && >+ elemDate.isSameOrAfter( >+ trailStart >+ ) && > elemDate.isSameOrBefore(trailEnd) > ) { > trailDisable = true; >@@ -1278,6 +1367,25 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > ); > }); > >+ // Additional check for hovering directly on existing booking's lead/trail periods >+ // If hovering on an existing booking's lead period when selecting start date, block selection >+ if ( >+ !periodPicker.selectedDates[0] && >+ target.classList.contains("existingBookingLead") >+ ) { >+ leadDisable = true; >+ } >+ >+ // If hovering on an existing booking's trail period when selecting end date, block selection >+ if ( >+ periodPicker.selectedDates[0] && >+ target.classList.contains( >+ "existingBookingTrail" >+ ) >+ ) { >+ trailDisable = true; >+ } >+ > if (leadDisable) { > target.classList.add("leadDisable"); > } >@@ -1291,6 +1399,241 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { > true > ); > } >+ >+ // Update feedback message >+ const feedbackDiv = >+ periodPicker.calendarContainer.querySelector( >+ ".booking-conflict-feedback" >+ ); >+ if (feedbackDiv) { >+ let message = ""; >+ let messageType = "info"; // info, warning, error >+ >+ // Determine what the hovered date is (needed for both error and info messages) >+ const today = dayjs().startOf("day"); >+ const isDisabled = >+ target.classList.contains( >+ "flatpickr-disabled" >+ ); >+ const isInExistingLead = >+ target.classList.contains( >+ "existingBookingLead" >+ ); >+ const isInExistingTrail = >+ target.classList.contains( >+ "existingBookingTrail" >+ ); >+ >+ // Generate appropriate feedback messages based on conflicts >+ if (leadDisable || trailDisable) { >+ messageType = "error"; >+ >+ // When selecting START date (no date selected yet) >+ if (!periodPicker.selectedDates[0]) { >+ // Check direct state first (what IS this date?) >+ if (hoverDate.isBefore(today)) { >+ message = __( >+ "Cannot select: date is in the past" >+ ); >+ } else if (isDisabled) { >+ message = __( >+ "Cannot select: this date is part of an existing booking" >+ ); >+ } else if (isInExistingLead) { >+ message = __( >+ "Cannot select: this date is part of an existing booking's lead period" >+ ); >+ } else if (isInExistingTrail) { >+ message = __( >+ "Cannot select: this date is part of an existing booking's trail period" >+ ); >+ } >+ // Then check calculated lead period conflicts >+ else if ( >+ leadDays > 0 && >+ leadStart.isSameOrBefore(today) >+ ) { >+ message = >+ __("Cannot select") + >+ ": " + >+ __( >+ "insufficient lead time (%s days required before start)" >+ ).format(leadDays); >+ } else if (leadDays > 0) { >+ // Use mathematical conflict detection (works across month boundaries) >+ if (leadConflictReason.withTrail) { >+ message = >+ __("Cannot select") + >+ ": " + >+ __( >+ "lead period (%s days before start) conflicts with an existing booking's trail period" >+ ).format(leadDays); >+ } else if ( >+ leadConflictReason.withLead >+ ) { >+ message = >+ __("Cannot select") + >+ ": " + >+ __( >+ "lead period (%s days before start) conflicts with an existing booking's lead period" >+ ).format(leadDays); >+ } else if ( >+ leadConflictReason.withBooking >+ ) { >+ message = >+ __("Cannot select") + >+ ": " + >+ __( >+ "lead period (%s days before start) conflicts with an existing booking" >+ ).format(leadDays); >+ } >+ } else { >+ message = __( >+ "Cannot select: conflicts with an existing booking" >+ ); >+ } >+ } >+ // When selecting END date (start date already selected) >+ else if (periodPicker.selectedDates[0]) { >+ // Check direct state first (what IS this date?) >+ if (isDisabled) { >+ message = __( >+ "Cannot select: this date is part of an existing booking" >+ ); >+ } else if (isInExistingLead) { >+ message = __( >+ "Cannot select: this date is part of an existing booking's lead period" >+ ); >+ } else if (isInExistingTrail) { >+ message = __( >+ "Cannot select: this date is part of an existing booking's trail period" >+ ); >+ } >+ // Then check calculated trail period conflicts >+ else if (trailDays > 0) { >+ // Use mathematical conflict detection (works across month boundaries) >+ if (trailConflictReason.withLead) { >+ message = >+ __("Cannot select") + >+ ": " + >+ __( >+ "trail period (%s days after return) conflicts with an existing booking's lead period" >+ ).format(trailDays); >+ } else if ( >+ trailConflictReason.withTrail >+ ) { >+ message = >+ __("Cannot select") + >+ ": " + >+ __( >+ "trail period (%s days after return) conflicts with an existing booking's trail period" >+ ).format(trailDays); >+ } else if ( >+ trailConflictReason.withBooking >+ ) { >+ message = >+ __("Cannot select") + >+ ": " + >+ __( >+ "trail period (%s days after return) conflicts with an existing booking" >+ ).format(trailDays); >+ } >+ } else { >+ message = __( >+ "Cannot select: conflicts with existing an booking" >+ ); >+ } >+ } >+ } else { >+ // Show helpful info messages when no conflicts >+ if (!periodPicker.selectedDates[0]) { >+ // When selecting start date, show both lead and trail info >+ if (leadDays > 0 && trailDays > 0) { >+ message = >+ __("Selecting start date") + >+ ". " + >+ __( >+ "Lead period: %s days before start" >+ ).format(leadDays) + >+ ". " + >+ __( >+ "Trail period: %s days after return" >+ ).format(trailDays); >+ } else if (leadDays > 0) { >+ message = >+ __("Selecting start date") + >+ ". " + >+ __( >+ "Lead period: %s days before start" >+ ).format(leadDays); >+ } else if (trailDays > 0) { >+ message = >+ __("Selecting start date") + >+ ". " + >+ __( >+ "Trail period: %s days after return" >+ ).format(trailDays); >+ } else { >+ message = __( >+ "Selecting start date" >+ ); >+ } >+ messageType = "info"; >+ } else { >+ if (trailDays > 0) { >+ message = >+ __("Selecting end date") + >+ ". " + >+ __( >+ "Trail period: %s days after return" >+ ).format(trailDays); >+ } else { >+ message = __("Selecting end date"); >+ } >+ messageType = "info"; >+ } >+ >+ // Show additional context if hovering over existing booking periods >+ if (isInExistingLead) { >+ message += >+ " ⢠" + >+ __( >+ "hovering existing booking's lead period" >+ ); >+ } else if (isInExistingTrail) { >+ message += >+ " ⢠" + >+ __( >+ "hovering existing booking's trail period" >+ ); >+ } >+ } >+ >+ feedbackDiv.textContent = message; >+ feedbackDiv.classList.remove( >+ "alert-danger", >+ "alert-warning", >+ "alert-info" >+ ); >+ >+ if (message) { >+ feedbackDiv.classList.remove("d-none"); >+ // Apply appropriate Bootstrap alert class based on message type >+ if (messageType === "error") { >+ feedbackDiv.classList.add( >+ "alert-danger" >+ ); >+ } else if (messageType === "warning") { >+ feedbackDiv.classList.add( >+ "alert-warning" >+ ); >+ } else { >+ feedbackDiv.classList.add("alert-info"); >+ } >+ } else { >+ feedbackDiv.classList.add("d-none"); >+ } >+ } > } > } > ); >-- >2.52.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37707
:
182718
|
190859
|
190860
|
190861
|
190862
|
191000
|
191001
|
191002
|
191003
|
191232
|
191233
|
191350
|
191351
|
191352
|
191353
|
191354
|
191355
|
191356
|
191357
|
191358
|
191359
|
191360
|
191361
|
191394
|
191395
|
191746
|
192006
|
192007
|
192008
|
192009
|
192010
|
192800
|
192801
|
192802
|
192803
|
192804
|
192805
|
192806
|
192807
|
192863
|
192864
|
192865
|
192866
|
192867
|
192868
|
192869
|
192870