Add event date to popup in map by apodacaduron · Pull Request #175 · sjacorg/bayanat

apodacaduron · 2025-09-01T18:11:45Z

Jira Issue

BYNT-1440

Description

Tweaks to event popup ux in map.

Checklist

Tests added/updated
Documentation updated (if needed)
New strings prepared for translations

API Changes (if applicable)

Permissions checked
Endpoint tests added

Additional Notes

[Any other relevant information]

Summary by CodeRabbit

New Features
- Richer map popups: display event number, parent ID, event type, address, date range with icons, and an estimated-timing indicator.
- One-click “Copy coordinates” button with on-screen confirmation.
UX Improvements
- Dates at exactly midnight now show as date-only for clearer readability.
- Clearer, multi-line popup layout for easier scanning.
Localization
- Added translations for “Copied to clipboard” and “Copy coordinates.”

…440-Add-event-date-to-popup-in-map

coderabbitai · 2025-09-05T16:29:43Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch BYNT-1440-Add-event-date-to-popup-in-map

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

tarekio · 2025-09-05T21:23:34Z

@coderabbitai review

coderabbitai · 2025-09-05T21:23:41Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (2)

enferno/static/js/common/config.js (1)
546-548: Expose event dates safely; consider normalizing nulls

Add small normalization to avoid undefined leaks (use null coalescing).

No blocking issues otherwise.
-        x.location.from_date = x.from_date;
-        x.location.to_date = x.to_date;
-        x.location.estimated = Boolean(x.estimated);
+        x.location.from_date = x.from_date ?? null;
+        x.location.to_date = x.to_date ?? null;
+        x.location.estimated = Boolean(x.estimated);
enferno/static/js/components/GlobalMap.js (1)
42-44: Avoid global mutation for snack function

Assigning window.showSnack creates a global footgun and cross-component coupling.
-  mounted() {
-    window.showSnack = this.$root.showSnack;
+  mounted() {
+    // Prefer a scoped helper stored on this instance
+    this._showSnack = (msg) => this.$root.showSnack(msg);
And update inline usage below to call a stable handler (see next comment for removing inline onclick altogether).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge Base: Disabled due to data retention organization setting

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 524c3e6 and 332fd1b.

📒 Files selected for processing (4)

enferno/admin/templates/admin/jsapi.jinja2 (1 hunks)
enferno/static/js/common/config.js (1 hunks)
enferno/static/js/components/GlobalMap.js (2 hunks)
enferno/static/js/mixins/global-mixin.js (2 hunks)

🔇 Additional comments (2)

enferno/static/js/mixins/global-mixin.js (1)

85-90: Hide midnight time: good; verify TZ semantics

Looks good. Midnight check happens after UTC/local/tz adjustments, which is correct. Verify this still yields dates (no time) for midnight timestamps across UTC vs local.

enferno/admin/templates/admin/jsapi.jinja2 (1)

29-30: Translations added for clipboard UX

LGTM. Keys are consistent with naming and used in GlobalMap. Ensure these get pulled into all locales.

coderabbitai · 2025-09-05T21:28:44Z

enferno/static/js/components/GlobalMap.js

    generatePopupContent(loc) {
-      function renderIfExists(label, value) {
-        return value ? `${label ? `<b>${label}:</b>` : ''} ${value}` : ''
+      const t = this.translations
+
+      // Dates (use mdi-calendar-clock)
+      const dates = []
+      if (loc.from_date) {
+        dates.push(`<span class="chip">
+      ${this.$root.formatDate(loc.from_date)}
+    </span>`)
      }
-      // Simple HTML structure for popup content. Adjust as needed.
-      return `<div class="popup-content">
-      <h4>${renderIfExists('', loc.title)}</h4>
-      <div>${renderIfExists(this.translations.number_, loc.number)} ${renderIfExists(this.translations.parentId_, loc.parentId)}</div>
-      <div>${renderIfExists(this.translations.coordinates_, `${loc.lat?.toFixed(6)}, ${loc.lng?.toFixed(6)}`)}</div>
-      <div>${renderIfExists('', loc.full_string)}</div>
-      ${loc.main ? `<div>${this.translations.mainIncident_}</div>` : ''}
-      <div>${renderIfExists(this.translations.type_, loc.geotype?.title)}</div>
-      <div>${renderIfExists(this.translations.eventType_, loc.eventtype)}</div>
-    </div>`;
+      if (loc.to_date) {
+        dates.push(`<span><i class="mdi mdi-arrow-right mr-1"></i>${this.$root.formatDate(loc.to_date)}</span>`)
+      }
+
+      // Estimated → calendar-question icon w/ tooltip
+      const estimated = loc.estimated
+        ? `<i class="mdi mdi-calendar-question mr-1" title="${loc.comment || t.timingForThisEventIsEstimated_}"></i>`
+        : '<i class="mdi mdi-calendar-clock mr-1"></i>'
+
+      const dateRange = dates.length
+        ? `<div class="d-flex">${estimated} ${dates.join(' ')}</div>`
+        : ''
+
+      // ParentId check
+      const parentId =
+        loc?.parent?.id
+          ? `<div>#${loc.parent?.id}</div>`
+          : ''
+
+      // Copy button for coordinates (added next to location)
+      const copyCoordsBtn =
+        loc.lat && loc.lng
+          ? `<button class="ml-1" title="${t.copyCoordinates_}" onclick="navigator.clipboard.writeText('${loc.lat.toFixed(6)}, ${loc.lng.toFixed(6)}'); window.showSnack('${translations.copiedToClipboard_}')">
+          <i class="mdi mdi-content-copy"></i>
+        </button>`
+          : ''
+
+      // Event type with icon (assuming eventtype.icon available)
+      const eventType = loc.eventtype
+        ? `<div class="d-flex align-center">
+         <i class="mdi mdi-calendar-range mr-1"></i>${loc.eventtype || ''}
+       </div>`
+        : ''
+
+      // Event type with icon (assuming eventtype.icon available)
+      const locNumber = loc.number
+        ? `<div class="d-flex align-center">
+         #${loc.number || ''}
+       </div>`
+        : ''
+
+      return `<div class="popup-content d-flex flex-column ga-1">
+      <div class="d-flex ga-2">${locNumber} ${parentId} ${eventType}</div>
+      <h4>${loc.title || ''}</h4>
+        <div class="d-flex align-center">
+          <i class="mdi mdi-map-marker-outline mr-1"></i>
+          ${loc.full_string || ''}
+          ${copyCoordsBtn}
+        </div>
+        ${loc.main ? `<div>${t.mainIncident_}</div>` : ''}
+        ${dateRange}
+      </div>`
    },


⚠️ Potential issue

XSS risk: HTML string assembly with unescaped user content

loc.title, loc.full_string, loc.eventtype, loc.comment are interpolated directly into HTML/attributes. If any are user-controlled, this is an XSS vector.

Apply escaping and avoid inline event handlers:

generatePopupContent(loc) { - const t = this.translations + const t = this.translations + const escapeHtml = (s) => s == null ? '' : String(s).replace(/[&<>"']/g, m => ({'&':'&','<':'<','>':'>','"':'"',"'":'''}[m])); // Dates (use mdi-calendar-clock) const dates = [] if (loc.from_date) { - dates.push(`<span class="chip"> - ${this.$root.formatDate(loc.from_date)} - </span>`) + dates.push(`<span class="chip">${escapeHtml(this.$root.formatDate(loc.from_date))}</span>`) } if (loc.to_date) { - dates.push(`<span><i class="mdi mdi-arrow-right mr-1"></i>${this.$root.formatDate(loc.to_date)}</span>`) + dates.push(`<span><i class="mdi mdi-arrow-right mr-1"></i>${escapeHtml(this.$root.formatDate(loc.to_date))}</span>`) } // Estimated → calendar-question icon w/ tooltip - const estimated = loc.estimated - ? `<i class="mdi mdi-calendar-question mr-1" title="${loc.comment || t.timingForThisEventIsEstimated_}"></i>` - : '<i class="mdi mdi-calendar-clock mr-1"></i>' + const estimated = loc.estimated + ? `<i class="mdi mdi-calendar-question mr-1" title="${escapeHtml(loc.comment || t.timingForThisEventIsEstimated_)}"></i>` + : '<i class="mdi mdi-calendar-clock mr-1"></i>' const dateRange = dates.length ? `<div class="d-flex">${estimated} ${dates.join(' ')}</div>` : '' // ParentId check - const parentId = - loc?.parent?.id - ? `<div>#${loc.parent?.id}</div>` - : '' + const parentId = (loc.parentId ?? loc?.parent?.id) != null + ? `<div>#${escapeHtml(String(loc.parentId ?? loc.parent?.id))}</div>` + : '' // Copy button for coordinates (added next to location) - const copyCoordsBtn = - loc.lat && loc.lng - ? `<button class="ml-1" title="${t.copyCoordinates_}" onclick="navigator.clipboard.writeText('${loc.lat.toFixed(6)}, ${loc.lng.toFixed(6)}'); window.showSnack('${translations.copiedToClipboard_}')"> - <i class="mdi mdi-content-copy"></i> - </button>` - : '' + const copyCoordsBtn = + Number.isFinite(loc.lat) && Number.isFinite(loc.lng) + ? `<button type="button" class="ml-1" title="${escapeHtml(t.copyCoordinates_)}" data-copy="${loc.lat.toFixed(6)}, ${loc.lng.toFixed(6)}"> + <i class="mdi mdi-content-copy" aria-hidden="true"></i> + <span class="sr-only">${escapeHtml(t.copyCoordinates_)}</span> + </button>` + : '' // Event type with icon (assuming eventtype.icon available) const eventType = loc.eventtype ? `<div class="d-flex align-center"> - <i class="mdi mdi-calendar-range mr-1"></i>${loc.eventtype || ''} + <i class="mdi mdi-calendar-range mr-1"></i>${escapeHtml(loc.eventtype || '')} </div>` : '' // Event type with icon (assuming eventtype.icon available) const locNumber = loc.number ? `<div class="d-flex align-center"> - #${loc.number || ''} + #${escapeHtml(String(loc.number || ''))} </div>` : '' - return `<div class="popup-content d-flex flex-column ga-1"> + const html = `<div class="popup-content d-flex flex-column ga-1"> <div class="d-flex ga-2">${locNumber} ${parentId} ${eventType}</div> - <h4>${loc.title || ''}</h4> + <h4>${escapeHtml(loc.title || '')}</h4> <div class="d-flex align-center"> <i class="mdi mdi-map-marker-outline mr-1"></i> - ${loc.full_string || ''} + ${escapeHtml(loc.full_string || '')} ${copyCoordsBtn} </div> ${loc.main ? `<div>${t.mainIncident_}</div>` : ''} ${dateRange} - </div>` + </div>`; + + // Turn string into DOM to attach a safe click handler + const wrapper = document.createElement('div'); + wrapper.innerHTML = html; + const btn = wrapper.querySelector('button[data-copy]'); + if (btn) { + btn.addEventListener('click', () => { + navigator.clipboard.writeText(btn.getAttribute('data-copy')) + .then(() => this.$root.showSnack(translations.copiedToClipboard_)) + .catch(() => this.$root.showSnack(translations.oops_)); + }); + } + return wrapper; },

Notes:

Returns a DOM node (Leaflet bindPopup accepts DOM elements), avoiding inline JS.

Escapes all user-facing strings and attribute values.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In enferno/static/js/components/GlobalMap.js around lines 71 to 133, the popup HTML is built by concatenating unescaped loc fields into strings and uses inline onclick, causing an XSS risk; instead, build and return a DOM element (Leaflet accepts nodes) and set all user-controlled values via textContent or setAttribute with proper escaping, remove inline event handlers (attach event listeners to buttons after creating the node), and avoid injecting raw loc.comment/loc.title/loc.full_string/eventtype into HTML — use sanitized or escaped values and .textContent, for coordinates put the raw values in a data- attribute and attach a click handler that reads data- attributes and uses navigator.clipboard, and ensure tooltip/title attributes are safely escaped or created via element.title rather than string interpolation.

coderabbitai · 2025-09-05T21:28:44Z

enferno/static/js/components/GlobalMap.js

+      const parentId =
+        loc?.parent?.id
+          ? `<div>#${loc.parent?.id}</div>`
+          : ''
+


⚠️ Potential issue

Parent ID lookup uses wrong property

You assign parentId in the aggregators, not parent.id. Use loc.parentId (fallback to loc.parent?.id only if present).

See fix in the previous diff.

🤖 Prompt for AI Agents

In enferno/static/js/components/GlobalMap.js around lines 95 to 99, the code builds parentId from loc.parent?.id but the correct source is loc.parentId (use loc.parent?.id only as a fallback when parentId is missing); update the assignment to prefer loc.parentId and fall back to loc.parent?.id, and ensure this change is applied where aggregators compute parentId (not using parent.id) so the displayed value uses the correct property.

coderabbitai · 2025-09-05T21:28:44Z

enferno/static/js/components/GlobalMap.js

+      const copyCoordsBtn =
+        loc.lat && loc.lng
+          ? `<button class="ml-1" title="${t.copyCoordinates_}" onclick="navigator.clipboard.writeText('${loc.lat.toFixed(6)}, ${loc.lng.toFixed(6)}'); window.showSnack('${translations.copiedToClipboard_}')">
+          <i class="mdi mdi-content-copy"></i>
+        </button>`
+          : ''
+


🛠️ Refactor suggestion

Copy button skips valid 0 coordinates and swallows clipboard errors

0 is falsy; current check hides the button at lat/lng = 0.

Unhandled clipboard rejections degrade UX on HTTP or denied permissions.

See robust check and promise handling in the previous diff (Number.isFinite + then/catch).

🤖 Prompt for AI Agents

In enferno/static/js/components/GlobalMap.js around lines 101 to 107, the copy-coordinates button currently hides when lat or lng is 0 and does not handle clipboard promise rejections; change the presence check to use Number.isFinite(loc.lat) && Number.isFinite(loc.lng) so 0 is treated as valid, and replace the inline onclick clipboard call with navigator.clipboard.writeText(...).then(() => window.showSnack(translations.copiedToClipboard_)).catch(err => { console.error(err); window.showSnack(translations.copyFailed_ || 'Copy failed'); }) to surface errors and avoid unhandled rejections (ensure you add or reuse a translation key for copy failure if needed).

… each is coming from

…440-Add-event-date-to-popup-in-map

…ent instead of inline js

level09

All good , just small improvement can be added.

level09 · 2025-09-12T11:07:59Z

enferno/static/js/components/GlobalMap.js

+
+      const btn = wrapper.querySelector('button[data-copy]');
+      if (btn) {
+        btn.addEventListener('click', () => {


Small improvement:

Clipboard copy can fail on non localhost, if user don't grant permissions or has some security settings / non https etc .. , right now this fails silently with this console error:

GlobalMap.js:117 Uncaught TypeError: Cannot read properties of undefined (reading 'writeText') at HTMLButtonElement.<anonymous> (GlobalMap.js:117:31)

let's just handle that case

.catch(() => this.$root.showSnack('Failed to copy coordinates'));

Let's address in another PR, merging this one for now.

add dates to map popup

4844315

apodacaduron requested a review from tarekio September 1, 2025 18:11

apodacaduron self-assigned this Sep 1, 2025

tarekio added the project m label Sep 4, 2025

apodacaduron added 2 commits September 5, 2025 08:27

Merge branch 'main' of https://github.com/sjacorg/bayanat into BYNT-1…

13abcb8

…440-Add-event-date-to-popup-in-map

improvements to event popup on map

0802e3c

apodacaduron added 2 commits September 5, 2025 13:08

if date has time 00:00 then hide time

02f42f3

display snack right after copy

332fd1b

tarekio approved these changes Sep 5, 2025

View reviewed changes

coderabbitai bot reviewed Sep 5, 2025

View reviewed changes

fix missing this. keyword

7e435f9

tarekio requested a review from level09 September 6, 2025 20:36

apodacaduron added 6 commits September 8, 2025 11:21

fix issue of map not rendering bulletin events

0a49deb

make sure to load both actors and bulletins into incidents map

70573cd

add class type property to aggregated locations so we know from where…

2f696d9

… each is coming from

fix parent id property and add icon

7489f38

add property use parent id

e051290

Merge branch 'main' of https://github.com/sjacorg/bayanat into BYNT-1…

fa14b37

…440-Add-event-date-to-popup-in-map

apodacaduron requested a review from tarekio September 8, 2025 18:48

apodacaduron added 5 commits September 8, 2025 16:19

install dom purify

93b2249

use dompurify to sanitize html and add events directly to button elem…

4f015ff

…ent instead of inline js

remove global snack and fix copy button coordinates if they are 0,0

78079ce

add nulls to dates if they dont exist

18277c5

remove dom purify

845f889

tarekio approved these changes Sep 11, 2025

View reviewed changes

apodacaduron added 2 commits September 11, 2025 15:01

remove global format change in case date is T00:00

8a4575b

hide time in popup if date is T00:00

ee8d78a

apodacaduron added 2 commits September 11, 2025 18:09

add arrow icon if the 2 dates are present

1e3fae2

fix typo and display arrow only if the 2 dates are present

f2fbc3e

level09 approved these changes Sep 12, 2025

View reviewed changes

tarekio added 2 commits September 12, 2025 19:18

Merge branch 'main' into BYNT-1440-Add-event-date-to-popup-in-map

a879231

Merge branch 'main' into BYNT-1440-Add-event-date-to-popup-in-map

7137a00

tarekio merged commit 4f4a0fc into main Sep 14, 2025
8 checks passed

tarekio deleted the BYNT-1440-Add-event-date-to-popup-in-map branch September 14, 2025 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add event date to popup in map#175

Add event date to popup in map#175
tarekio merged 23 commits intomainfrom
BYNT-1440-Add-event-date-to-popup-in-map

apodacaduron commented Sep 1, 2025 •

edited by tarekio

Loading

Uh oh!

coderabbitai bot commented Sep 5, 2025 •

edited

Loading

Review skipped

Uh oh!

tarekio commented Sep 5, 2025

Uh oh!

coderabbitai bot commented Sep 5, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Sep 5, 2025

Uh oh!

coderabbitai bot Sep 5, 2025

Uh oh!

coderabbitai bot Sep 5, 2025

Uh oh!

level09 left a comment

Uh oh!

level09 Sep 12, 2025

Uh oh!

tarekio Sep 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

apodacaduron commented Sep 1, 2025 • edited by tarekio Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jira Issue

Description

Checklist

API Changes (if applicable)

Additional Notes

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

tarekio commented Sep 5, 2025

Uh oh!

coderabbitai bot commented Sep 5, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

level09 left a comment

Choose a reason for hiding this comment

Uh oh!

level09 Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

tarekio Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

apodacaduron commented Sep 1, 2025 •

edited by tarekio

Loading

coderabbitai bot commented Sep 5, 2025 •

edited

Loading