Show beacon errors in a dialog#385
Conversation
Add a dedicated error dialog to the beacon map page so location sharing failures are surfaced to users in the UI instead of being visible only in the console. Update the beacon map Stimulus controller to open the dialog for initial map load failures, geolocation failures, publish/update failures, and stop-sharing failures while keeping background polling refresh errors silent. Prefer backend error messages when available, map geolocation permission denials to the existing localized message, and add a localized dialog title for the new UI.
There was a problem hiding this comment.
Pull request overview
Adds a user-facing error modal to the Beacon Map page so operational failures (map load, geolocation, publish/update, stop sharing) are surfaced in the UI rather than only via console logs.
Changes:
- Add localized
map.error_titlestrings (EN/JA) for the new error dialog. - Render an error
<dialog>on the map page and wire it to the Stimulus controller via targets. - Update
beacon_map_controller.jsto open/close the dialog on key failure paths (while keeping polling refresh failures silent).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| config/locales/en.yml | Adds map.error_title for the dialog heading. |
| config/locales/ja.yml | Adds map.error_title for the dialog heading. |
| app/views/maps/show.html.erb | Introduces the error <dialog> markup and targets for message rendering. |
| app/javascript/controllers/beacon_map_controller.js | Adds dialog targets + logic to show the modal on load/geolocation/publish/stop errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (this.hasErrorDialogTarget) { | ||
| this.errorDialogTarget.addEventListener('close', () => { | ||
| document.documentElement.classList.remove('overflow-hidden') | ||
| }) | ||
| } |
There was a problem hiding this comment.
connect() adds a close event listener with an inline arrow function but never removes it. If this controller reconnects (e.g., via Turbo navigation), multiple listeners can accumulate and run repeatedly on each close. Consider using a Stimulus data-action="close->beacon-map#closeErrorDialog" on the <dialog>, or store the handler on this and remove it in disconnect().
| messageForError(error, fallbackMessage) { | ||
| if (error?.message) { | ||
| return error.message | ||
| } | ||
|
|
||
| return fallbackMessage | ||
| } |
There was a problem hiding this comment.
messageForError() always prefers error.message, which means browser/network/Google Maps loader errors will surface as hard-coded or non-localized strings (e.g., the Google Maps script onerror message, or fetch's "Failed to fetch") instead of the provided localized fallback. To keep the dialog user-friendly and localized, only surface backend-provided messages (e.g., those created in request() from body.errors) and otherwise return the fallback message.
| <dialog data-beacon-map-target="errorDialog" class="border-0 bg-transparent p-0 backdrop:bg-black/50"> | ||
| <div class="fixed inset-0 flex items-center justify-center"> | ||
| <div class="flex max-h-[90vh] w-[90vw] max-w-[480px] flex-col rounded-lg bg-white"> | ||
| <div class="border-b px-4 py-2"> | ||
| <h2 class="text-xl font-bold"><%= t('map.error_title') %></h2> |
There was a problem hiding this comment.
The error dialog lacks an accessible name. Without aria-label/aria-labelledby, many screen readers will announce an unlabeled dialog, even though a visible <h2> exists. Add an id to the heading and set aria-labelledby on the <dialog> (or use aria-label) so the modal is correctly announced.
| <dialog data-beacon-map-target="errorDialog" class="border-0 bg-transparent p-0 backdrop:bg-black/50"> | |
| <div class="fixed inset-0 flex items-center justify-center"> | |
| <div class="flex max-h-[90vh] w-[90vw] max-w-[480px] flex-col rounded-lg bg-white"> | |
| <div class="border-b px-4 py-2"> | |
| <h2 class="text-xl font-bold"><%= t('map.error_title') %></h2> | |
| <dialog data-beacon-map-target="errorDialog" aria-labelledby="map-error-dialog-title" class="border-0 bg-transparent p-0 backdrop:bg-black/50"> | |
| <div class="fixed inset-0 flex items-center justify-center"> | |
| <div class="flex max-h-[90vh] w-[90vw] max-w-[480px] flex-col rounded-lg bg-white"> | |
| <div class="border-b px-4 py-2"> | |
| <h2 id="map-error-dialog-title" class="text-xl font-bold"><%= t('map.error_title') %></h2> |
Add a dedicated error dialog to the beacon map page so location sharing failures are surfaced to users in the UI instead of being visible only in the console.
Update the beacon map Stimulus controller to open the dialog for initial map load failures, geolocation failures, publish/update failures, and stop-sharing failures while keeping background polling refresh errors silent.
Prefer backend error messages when available, map geolocation permission denials to the existing localized message, and add a localized dialog title for the new UI.