Skip to content

Conversation

@Ilnur-Su
Copy link

@Ilnur-Su Ilnur-Su commented Oct 1, 2025


This Pull request Fixes #1184

Adding rock search and selection functions.

Type of PR.

  • refactor
  • feature
  • bug fix
  • documentation
  • optimization
  • other

Description

Added search and selection functionality for crags in the map toolbar:

  • Input field for searching crags by areaName
  • Dropdown suggestions with clickable results
  • Selecting a crag highlights it and moves the map to its position
  • Improved UX for navigating crags directly from the toolbar

What this PR achieves

  • Introduced allCrags state populated from map features
  • Added highlightCrag function to select and fly to a crag
  • Extended MapToolbar props (cragsList, onSelectCrag)
  • Implemented search with live suggestions in MapToolbar

Screenshots

Search and Suggestions

Снимок экрана 2025-10-02 в 01 30 53

Marking a selected crag

Снимок экрана 2025-10-02 в 01 31 35

Copy link
Contributor

@mikeschen mikeschen left a comment

Choose a reason for hiding this comment

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

Some comments. I will try to test out sometime soonish, I am slammed with day job and contracting job which is why I havent reviewed sooner :(

if (!mapInstance) return
const features = mapInstance.querySourceFeatures('crags', { sourceLayer: 'crags' })
const cragsFeatures = features
.map(f => tileToFeature('crag-name-labels', {x:0,y:0}, f.geometry, f.properties as TileProps, mapInstance))
Copy link
Contributor

Choose a reason for hiding this comment

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

please use var name as "feature" rather than "f"

}

const filtered = cragsList.filter(c =>
c.data.areaName.toLowerCase().includes(value.toLowerCase())
Copy link
Contributor

Choose a reason for hiding this comment

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

please use "crag" rather than "c" for var name

/>
{suggestions.length > 0 && (
<ul className='absolute top-full left-0 w-full bg-white shadow-lg rounded-md max-h-48 overflow-y-auto z-50'>
{suggestions.map(c => (
Copy link
Contributor

Choose a reason for hiding this comment

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

please use "suggestion"rather than "c" for var name

const features = mapInstance.querySourceFeatures('crags', { sourceLayer: 'crags' })
const cragsFeatures = features
.map(f => tileToFeature('crag-name-labels', {x:0,y:0}, f.geometry, f.properties as TileProps, mapInstance))
.filter((f): f is ActiveFeature => f !== null) // фильтруем null и сохраняем тип
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover comment.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds search and selection functionality for crags in the map toolbar, allowing users to quickly find and navigate to specific climbing areas.

Key Changes:

  • Added search input with live filtering of crags by name
  • Implemented crag selection that highlights the selected crag and flies the map to its location
  • Extracted all crags from map features into state for search functionality

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/components/maps/MapToolbar.tsx Added search input, suggestions dropdown, and handlers for filtering and selecting crags
src/components/maps/GlobalMap.tsx Added state for all crags, extraction logic from map features, and highlight/fly-to function for selected crags

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const features = mapInstance.querySourceFeatures('crags', { sourceLayer: 'crags' })
const cragsFeatures = features
.map(f => tileToFeature('crag-name-labels', {x:0,y:0}, f.geometry, f.properties as TileProps, mapInstance))
.filter((f): f is ActiveFeature => f !== null) // фильтруем null и сохраняем тип
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

Comment is written in Russian ('фильтруем null и сохраняем тип' means 'filter null and preserve type'). Project comments should be in English for consistency. Replace with: 'filter out null values and preserve type'.

Suggested change
.filter((f): f is ActiveFeature => f !== null) // фильтруем null и сохраняем тип
.filter((f): f is ActiveFeature => f !== null) // filter out null values and preserve type

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +71
<div className='relative w-64'>
<input
type="text"
value={search}
onChange={handleSearchChange}
placeholder="Search crags..."
className='input input-bordered w-full'
/>
{suggestions.length > 0 && (
<ul className='absolute top-full left-0 w-full bg-white shadow-lg rounded-md max-h-48 overflow-y-auto z-50'>
{suggestions.map(c => (
<li
key={c.data.id}
onClick={() => handleSelect(c)}
className='cursor-pointer p-2 hover:bg-gray-200'
>
{c.data.areaName}
</li>
))}
</ul>
)}
</div>
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

Inconsistent indentation. This div has extra leading spaces compared to surrounding elements. It should align with the Checkbox components at the same nesting level.

Suggested change
<div className='relative w-64'>
<input
type="text"
value={search}
onChange={handleSearchChange}
placeholder="Search crags..."
className='input input-bordered w-full'
/>
{suggestions.length > 0 && (
<ul className='absolute top-full left-0 w-full bg-white shadow-lg rounded-md max-h-48 overflow-y-auto z-50'>
{suggestions.map(c => (
<li
key={c.data.id}
onClick={() => handleSelect(c)}
className='cursor-pointer p-2 hover:bg-gray-200'
>
{c.data.areaName}
</li>
))}
</ul>
)}
</div>
<div className='relative w-64'>
<input
type="text"
value={search}
onChange={handleSearchChange}
placeholder="Search crags..."
className='input input-bordered w-full'
/>
{suggestions.length > 0 && (
<ul className='absolute top-full left-0 w-full bg-white shadow-lg rounded-md max-h-48 overflow-y-auto z-50'>
{suggestions.map(c => (
<li
key={c.data.id}
onClick={() => handleSelect(c)}
className='cursor-pointer p-2 hover:bg-gray-200'
>
{c.data.areaName}
</li>
))}
</ul>
)}
</div>

Copilot uses AI. Check for mistakes.
Comment on lines +227 to +231
setClickInfo(prev => {
if (prev) setActiveFeatureVisual(prev, { selected: false, hover: false })
setActiveFeatureVisual(crag, { selected: true, hover: false })
return crag
})
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

The setClickInfo callback is using the previous state incorrectly. The callback receives prev but doesn't use it in the return logic - it always returns crag regardless of the previous value. This should likely be a direct call: setClickInfo(crag) followed by the visual state updates, or the callback should properly handle the state transition.

Suggested change
setClickInfo(prev => {
if (prev) setActiveFeatureVisual(prev, { selected: false, hover: false })
setActiveFeatureVisual(crag, { selected: true, hover: false })
return crag
})
if (clickInfo) setActiveFeatureVisual(clickInfo, { selected: false, hover: false })
setActiveFeatureVisual(crag, { selected: true, hover: false })
setClickInfo(crag)

Copilot uses AI. Check for mistakes.
Comment on lines +219 to +221
const features = mapInstance.querySourceFeatures('crags', { sourceLayer: 'crags' })
const cragsFeatures = features
.map(f => tileToFeature('crag-name-labels', {x:0,y:0}, f.geometry, f.properties as TileProps, mapInstance))
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

Magic values {x:0, y:0} are passed to tileToFeature without explanation. Consider extracting this as a named constant (e.g., DEFAULT_TILE_COORDS) or adding a comment explaining why these coordinates are used when querying all crags.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a search bar to the /maps route

2 participants