feat: make showing distance options in filters modal a toggleable prop#866
feat: make showing distance options in filters modal a toggleable prop#866mkouzel-yext merged 8 commits intomainfrom
Conversation
|
Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information. |
WalkthroughThe Locator component adds a new filters field Sequence Diagram(s)sequenceDiagram
participant App as LocatorComponent
participant Internal as LocatorInternal
participant Modal as FilterModal
participant Distance as DistanceFilter
participant Store as FilterState
App->>Internal: mount(filters { showDistanceOptions })
Internal->>Modal: open(filterProps + showDistanceOptions)
alt showDistanceOptions = true
Modal->>Distance: render DistanceFilter
Distance-->>Modal: selectedDistanceMiles
Modal->>Internal: applyFilter(selectedDistanceMiles)
Internal->>Store: merge Near filter (radius = selectedDistanceMiles)
else showDistanceOptions = false
Modal-->>Distance: not rendered
Modal->>Internal: applyFilter(no distance change)
Internal->>Store: merge Near filter (radius = previousRadius)
end
Note over Internal,Store: initial location updates and handleFilterSelect\nuse showDistanceOptions to decide whether to overwrite radius
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-10-29T22:00:03.843ZApplied to files:
🔇 Additional comments (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/components/Locator.tsx (1)
1007-1032: Note: selectedDistanceMiles state is disconnected from filter behavior.Line 1032 resets
selectedDistanceMilestoDEFAULT_RADIUS_MILES, but this value is no longer used when constructing the location filter (lines 1011-1015 omit the radius field). This state management appears to be preserved for when the distance filter UI is re-enabled in the future.Consider adding a comment explaining this intentional disconnection, or remove the state management at line 1032 if the feature won't be re-enabled.
searchActions.setStaticFilters( unaffectedStaticFilters.concat( updatedLocationFilters, updatedOpenNowFilters ) ); searchActions.resetFacets(); // Execute search to update AppliedFilters components searchActions.setOffset(0); executeSearch(searchActions); - setSelectedDistanceMiles(DEFAULT_RADIUS_MILES); + // Note: selectedDistanceMiles state preserved for future when distance filter is re-enabled + setSelectedDistanceMiles(DEFAULT_RADIUS_MILES); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/visual-editor/src/components/Locator.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 833
File: packages/visual-editor/src/components/Locator.tsx:1050-1057
Timestamp: 2025-10-29T22:00:03.843Z
Learning: In packages/visual-editor/src/components/Locator.tsx, the AppliedFilters component is intentionally rendered in two locations (inside the filter modal and outside it) as per the design requirements. This dual rendering should not be flagged as a duplicate issue.
📚 Learning: 2025-10-29T22:00:03.843Z
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 833
File: packages/visual-editor/src/components/Locator.tsx:1050-1057
Timestamp: 2025-10-29T22:00:03.843Z
Learning: In packages/visual-editor/src/components/Locator.tsx, the AppliedFilters component is intentionally rendered in two locations (inside the filter modal and outside it) as per the design requirements. This dual rendering should not be flagged as a duplicate issue.
Applied to files:
packages/visual-editor/src/components/Locator.tsx
🔇 Additional comments (3)
packages/visual-editor/src/components/Locator.tsx (3)
697-699: LGTM: Preserves radius from filter search.The spread operator correctly preserves the radius (and other fields) from the filter search response, preventing unintended overwrites. This aligns with the PR objective.
1561-1617: LGTM: Clean implementation of conditional distance filter rendering.The
hiddenprop implementation is well-structured:
- Uses conditional rendering (
!hidden && ...) instead of CSS hiding, which is better for accessibility- Preserves all functionality for future enablement
- Includes accessible
aria-labelattributes on distance option buttons- Maintains state management infrastructure
The approach correctly hides the feature while keeping the implementation ready for future activation.
791-795: The review comment contradicts itself and mischaracterizes the code changes.The snippet provided shows
radius: selectedDistanceMiles * MILES_TO_METERS,is still present at line 794. The claim that "radius field removed from filter value" is incorrect—the radius is not removed in the final code state. The request to verify API behavior without radius is based on a false premise and should not be acted upon.Likely an incorrect or invalid review comment.
mkouzel-yext
force-pushed
the
hide-distance-options-locator
branch
from
e3b165a to
e0c1ad2
Compare
mkouzel-yext
changed the title
mkouzel-yext
changed the title
The distance options were included in the mocks but not actually intended to be implemented yet. For now we'll hide them, and decide whether to keep or remove later. Updates filter select and clear all handlers to not overwrite the radius found by filter search with the selected distance. J=WAT-5141 TEST=manual jam:https://jam.dev/c/0e818afa-73fc-4e79-afae-b0cf177169d3
This reverts commit 8a68aef.
Either true or false.
mkouzel-yext
force-pushed
the
hide-distance-options-locator
branch
from
6c8fc5c to
278e07d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/components/Locator.tsx (1)
1029-1040: Consider adding fallback for undefined radius.The conditional logic correctly implements the feature intent. However, if
previousValue?.radiusis undefined whenshowDistanceOptionsis false, the filter will have an undefined radius which could cause issues.While the initial search always sets a radius, consider adding a fallback for defensive coding:
const radius = showDistanceOptions ? DEFAULT_RADIUS_MILES * MILES_TO_METERS - : previousValue?.radius; + : previousValue?.radius ?? DEFAULT_RADIUS_MILES * MILES_TO_METERS;This ensures the filter always has a valid radius value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/visual-editor/src/components/Locator.tsx(13 hunks)packages/visual-editor/src/docs/components.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 833
File: packages/visual-editor/src/components/Locator.tsx:1050-1057
Timestamp: 2025-10-29T22:00:03.843Z
Learning: In packages/visual-editor/src/components/Locator.tsx, the AppliedFilters component is intentionally rendered in two locations (inside the filter modal and outside it) as per the design requirements. This dual rendering should not be flagged as a duplicate issue.
📚 Learning: 2025-10-29T22:00:03.843Z
Learnt from: mkouzel-yext
Repo: yext/visual-editor PR: 833
File: packages/visual-editor/src/components/Locator.tsx:1050-1057
Timestamp: 2025-10-29T22:00:03.843Z
Learning: In packages/visual-editor/src/components/Locator.tsx, the AppliedFilters component is intentionally rendered in two locations (inside the filter modal and outside it) as per the design requirements. This dual rendering should not be flagged as a duplicate issue.
Applied to files:
packages/visual-editor/src/docs/components.mdpackages/visual-editor/src/components/Locator.tsx
🧬 Code graph analysis (1)
packages/visual-editor/src/components/Locator.tsx (1)
packages/visual-editor/src/editor/YextField.tsx (1)
YextField(163-284)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (11)
packages/visual-editor/src/docs/components.md (1)
431-435: LGTM!The documentation correctly reflects the new
showDistanceOptionsprop with clear description and proper table formatting.packages/visual-editor/src/components/Locator.tsx (10)
452-457: LGTM!The prop definition is clear with appropriate JSDoc documentation and correct type definition.
515-524: LGTM!The field definition follows established patterns and provides appropriate UI configuration for the toggle.
563-563: LGTM!The default value correctly sets
showDistanceOptionsto false, matching the PR objective to hide distance options by default.
610-610: LGTM!Proper destructuring of the new prop from the filters configuration.
708-720: Logic correctly implements feature with clear comment.The conditional radius logic properly preserves the filter search radius when distance options are disabled. The inline comment clearly explains the intent.
1077-1078: LGTM!Correctly updates the filter modal visibility logic to include the new distance options toggle.
1173-1173: LGTM!Correctly passes the
showDistanceOptionsprop down to the FilterModal component.
1462-1462: LGTM!The interface properly defines the new prop with a clear explanatory comment.
1475-1475: LGTM!Proper destructuring of the new prop within the FilterModal component.
1519-1524: LGTM!The conditional rendering correctly hides the DistanceFilter UI when
showDistanceOptionsis false, following the same pattern as the OpenNowFilter.
This file sets the defaultProps for when a new page group is created.
3 participants
Adds a field in the locator settings panel to show or not show distance options that set the radius of any applied near filters.
J=WAT-5141
TEST=manual
jam:https://jam.dev/c/9fe6b4f6-8df1-4871-816e-c88186482681