Skip to content

Add chat window#618

Merged
Lucaci-Andrei merged 84 commits intomainfrom
feat/add-new-chat-window
Jan 27, 2026
Merged

Add chat window#618
Lucaci-Andrei merged 84 commits intomainfrom
feat/add-new-chat-window

Conversation

@Lucaci-Andrei
Copy link
Copy Markdown
Contributor

@Lucaci-Andrei Lucaci-Andrei commented Jan 23, 2026

Summary by CodeRabbit

  • New Features
    • Conversational AI chat: chat window, input, messages, chat button, location consent popup, and "Ask with AI" from Search; chat can return locations and show routes.
  • Improvements
    • Chat integrated into main UI with conditional control hiding; map auto-fits chat results; improved mobile keyboard/layout handling; configurable bottom-sheet swipe/scroll; new chat UI styles.
  • Bug Fixes
    • Better null checks, IME/Enter handling, cleanup for scrolling and directions.

✏️ Tip: You can customize this high-level summary in your review settings.

… for better clarity and add floor change functionality based on valid locations
Copy link
Copy Markdown
Collaborator

@matbmapspeople matbmapspeople left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, remember chagelog and run build

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@packages/map-template/src/components/ChatWindow/ChatSearchResults/ChatSearchResults.jsx`:
- Around line 47-76: The handler handleLocationClick accesses
location.properties directly after using optional chaining, which can throw if
properties is undefined; update the function to defensively read const props =
location?.properties and guard before using props (e.g., return early or skip
venue/floor logic when props is falsy), replace uses of
location.properties.venueId and location.properties.floor with safe accesses
(props?.venueId, props?.floor) and only call setCurrentVenueName,
setIsLocationClicked, mapsIndoorsInstance.setFloor, and mapsIndoorsInstance.goTo
when props is present and values are valid; keep other calls like
setCurrentLocation and mapsIndoorsInstance.getFloor as-is but ensure
mapsIndoorsInstance exists before invoking its methods.
🧹 Nitpick comments (2)
packages/map-template/src/components/ChatWindow/ChatSearchResults/ChatSearchResults.jsx (2)

78-92: Unnecessary async on synchronous helpers.

getBottomPadding and getLeftPadding don't perform any asynchronous operations, yet they're declared async and awaited via Promise.all. This adds unnecessary overhead and obscures the synchronous nature of the logic.

♻️ Suggested simplification
-    /**
-     * Get bottom padding when selecting a location
-     */
-    const getBottomPadding = async () => {
-        if (isDesktop) {
-            return 0;
-        } else {
-            return 200;
-        }
-    };
-
-    /**
-     * Get left padding when selecting a location
-     */
-    const getLeftPadding = async () => 0;
+    const getBottomPadding = () => isDesktop ? 0 : 200;
+    const getLeftPadding = () => 0;

And update the usage:

-        const [bottomPadding, leftPadding] = await Promise.all([
-            getBottomPadding(),
-            getLeftPadding()
-        ]);
+        const bottomPadding = getBottomPadding();
+        const leftPadding = getLeftPadding();

113-123: Add i18n support for toggle button text to match project standards.

The toggle button contains hardcoded English strings while the rest of the application uses i18next for internationalization. Other components (Directions, LocationDetails, LegendDialog) follow the const { t } = useTranslation() pattern.

Implementation
 import { useState } from 'react';
+import { useTranslation } from 'react-i18next';
 import { useRecoilState, useRecoilValue } from 'recoil';

 const ChatSearchResults = ({ locations }) => {
+    const { t } = useTranslation();
     const [isExpanded, setIsExpanded] = useState(false);

Then update the button text (lines 119-120):

                 {isExpanded
-                    ? `Collapse all ${totalLocations} locations found`
-                    : `View all ${totalLocations} locations found`
+                    ? t('Collapse all {{count}} locations found', { count: totalLocations })
+                    : t('View all {{count}} locations found', { count: totalLocations })
                 }

Add the entries to all i18n language files in packages/map-template/src/i18n/.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@packages/map-template/src/components/ChatWindow/ChatWindow.jsx`:
- Around line 78-94: The fetchLocationsAndUpdateMessage callback should include
setChatHistory in its dependency array, avoid console.* calls, and use
Promise.allSettled instead of Promise.all so individual getLocation failures
don't abort all results; change the logic inside fetchLocationsAndUpdateMessage
(which calls window.mapsindoors.services.LocationsService.getLocation and then
updateLatestServerMessage) to await Promise.allSettled, keep only fulfilled
results, filter out null/failed entries, and call setChatHistory(prev =>
updateLatestServerMessage(prev, { locations: validLocations })); also remove the
console.log/console.error statements.
- Around line 110-122: The useEffect watching directionsLocationIds currently
logs to console and omits setChatHistory from its dependencies; remove the debug
console.log call and add setChatHistory to the dependency array so the effect
depends on both directionsLocationIds and setChatHistory, and ensure the effect
still calls setChatHistory(prev => updateLatestServerMessage(prev, {
directionsLocationIds, canShowRoute: true, routeDirections:
directionsLocationIds })) as before (refer to useEffect, setChatHistory,
updateLatestServerMessage, and directionsLocationIds).
- Around line 139-178: The current use of Date.now() (in ChatWindow.jsx when
creating userMessage and server messages) can produce collisions; replace those
inline Date.now() usages with a single robust ID generator function (e.g.,
generateMessageId) and call it for every message creation so IDs are unique
across async boundaries. Add a local helper generateMessageId that uses
crypto.randomUUID() if available (with a fallback like Date.now().toString() +
Math.random().toString(36)), and update the userMessage.id and the server
response objects (the places using Date.now(), Date.now() + 1, etc.) to call
generateMessageId() instead. Ensure every place that previously used Date.now()
for message ids in ChatWindow.jsx uses this helper so no two messages share an
id.
- Around line 27-39: The function updateLatestServerMessage currently uses
Array.prototype.findLastIndex (lastServerIndex) which is not supported in older
Safari; replace it with a modern-compatible approach such as iterating from the
end to find the last message with type === 'server' (e.g., a reverse for loop
that sets lastServerIndex) or compute lastServerIndex by using
messages.slice().reverse().findIndex(...) and converting the index back, or add
a polyfill for findLastIndex (core-js) or adjust build targets in
vite.config.js; update the logic that uses lastServerIndex and ensure
updatedMessages[lastServerIndex] assignment still occurs only when an index is
found.
🧹 Nitpick comments (3)
packages/map-template/src/components/ChatWindow/ChatWindow.jsx (3)

96-108: Remove debug logging.

The console.log on line 105 should be removed before merging.


190-190: Remove unused dependency.

mapsIndoorsInstance is listed in the dependency array but is not used within handleSendMessage. Remove it to avoid confusion and unnecessary re-renders.

-}, [isLoading, generateResponse, apiKey, userPosition, currentVenueName, mapsIndoorsInstance, locationShareConsent]);
+}, [isLoading, generateResponse, apiKey, userPosition, currentVenueName, locationShareConsent, setChatHistory]);

Note: setChatHistory is used but missing from the dependencies.


48-48: Unused ref.

chatWindowRef is created on line 48 and attached to the container div on line 280, but it's never used elsewhere in the component. Remove it if unnecessary, or document its intended future use.

Also applies to: 280-280

Comment thread packages/map-template/src/components/ChatWindow/ChatWindow.jsx
Comment thread packages/map-template/src/components/ChatWindow/ChatWindow.jsx
Comment thread packages/map-template/src/components/ChatWindow/ChatWindow.jsx
Comment thread packages/map-template/src/components/ChatWindow/ChatWindow.jsx
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/map-template/src/components/ChatWindow/ChatWindow.jsx`:
- Around line 128-192: handleSendMessage uses the setter setChatHistory multiple
times but it is not included in the useCallback dependency array; update the
dependency list for the handleSendMessage callback to include setChatHistory so
the hook correctly captures the latest setter reference (i.e., add
setChatHistory to the array alongside isLoading, generateResponse, apiKey,
userPosition, currentVenueName, mapsIndoorsInstance, locationShareConsent).
🧹 Nitpick comments (2)
packages/map-template/src/components/ChatWindow/ChatWindow.jsx (2)

46-46: Unused Recoil value mapsIndoorsInstance.

mapsIndoorsInstance is read from Recoil state on line 46 and included in handleSendMessage's dependency array (line 192), but it's never actually used in the function body. Either remove it or clarify its intended purpose.

♻️ Suggested fix
-    const mapsIndoorsInstance = useRecoilValue(mapsIndoorsInstanceState);

And remove from dependency array on line 192:

-    }, [isLoading, generateResponse, apiKey, userPosition, currentVenueName, mapsIndoorsInstance, locationShareConsent]);
+    }, [isLoading, generateResponse, apiKey, userPosition, currentVenueName, locationShareConsent]);

98-110: Remove debug logging.

The console.log on line 107 should be removed before merging.

♻️ Suggested fix
         // Only fetch location details if we have results
         if (searchResults && searchResults.length > 0) {
-            console.log('ChatWindow: Received search results from provider:', searchResults);
             fetchLocationsAndUpdateMessage(searchResults);
         }

Comment thread packages/map-template/src/components/ChatWindow/ChatWindow.jsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants