-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(20432): select ref area by user's location #966
feat(20432): select ref area by user's location #966
Conversation
WalkthroughThe pull request introduces enhancements to location-related functionality and user interface components. The changes span multiple files, focusing on improving reference area selection, adding user location retrieval, and updating localization strings. The modifications include making the geometry hash optional, adding location access methods, introducing new translation keys, and updating CSS styles to support new user interaction features. Changes
Sequence DiagramsequenceDiagram
participant User
participant ReferenceAreaInfo
participant UserLocation
participant Boundaries
participant ReferenceArea
User->>ReferenceAreaInfo: Select current location
ReferenceAreaInfo->>UserLocation: getUserLocation()
UserLocation-->>ReferenceAreaInfo: Return coordinates
ReferenceAreaInfo->>Boundaries: getBoundaries(coordinates)
Boundaries-->>ReferenceAreaInfo: Return boundary data
ReferenceAreaInfo->>ReferenceArea: updateReferenceArea()
ReferenceArea-->>ReferenceAreaInfo: Confirm update
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Bundle size diff
|
Preview environments for this PR:
These previews are automatically updated with each commit. Note: After a new deployment, it may take a few minutes for the changes to propagate and for caches to update. During this time, you might experience temporary loading issues or see an older version of the app. If the app fails to load, please wait a few minutes and try again. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #966 +/- ##
==========================================
- Coverage 10.04% 10.02% -0.03%
==========================================
Files 668 669 +1
Lines 29398 29459 +61
Branches 1235 1236 +1
==========================================
Hits 2954 2954
- Misses 25915 25975 +60
- Partials 529 530 +1
☔ View full report in Codecov by Sentry. |
Bundle ReportChanges will increase total bundle size by 4.0kB (0.07%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
src/core/shared_state/referenceArea.ts (1)
Line range hint
40-43
: Consider adding type guard for hash comparison.The hash comparison logic could be more type-safe by using a type guard.
// update only in case if geometry source or hash has changed const referenceAreaOld = ctx.get(referenceAreaAtom); - if (!referenceAreaOld || !referenceAreaOld.hash || referenceAreaOld.hash !== hash) { + const hasHashChanged = (old: GeometryWithHash | null): boolean => { + return !old?.hash || old.hash !== hash; + }; + if (!referenceAreaOld || hasHashChanged(referenceAreaOld)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/core/focused_geometry/types.ts
(1 hunks)src/core/localization/translations/en/common.json
(2 hunks)src/core/shared_state/referenceArea.ts
(1 hunks)src/features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.module.css
(3 hunks)src/features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.tsx
(3 hunks)src/utils/common/userLocation.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.tsx
[error] 115-118: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form
element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (7)
src/features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.module.css (3)
7-11
: LGTM! Well-structured layout and typography.The flex layout and typography settings are properly implemented using design system variables.
22-22
: LGTM! Consistent spacing and styling.The use of design system variables for spacing and colors maintains visual consistency.
Also applies to: 29-29
49-56
: LGTM! Clean loading indicator implementation.The loading animation is well-implemented using keyframes and pseudo-elements, following best practices.
src/utils/common/userLocation.ts (1)
10-11
: 🧹 Nitpick (assertive)Verify the coordinate order convention.
The function returns coordinates in [longitude, latitude] order, which is common for GeoJSON but might be confusing for developers expecting [latitude, longitude]. Consider adding a comment or documentation to clarify this convention.
(position) => { const { latitude, longitude } = position.coords; + // Using [longitude, latitude] order to match GeoJSON specification resolve([longitude, latitude]); },
✅ Verification successful
The [longitude, latitude] coordinate order is correct and follows GeoJSON specification
The coordinate order is consistent with GeoJSON specification and is used consistently throughout the codebase. Adding the suggested documentation comment would indeed improve code clarity for developers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for coordinate-related patterns and GeoJSON usage echo "=== Searching for coordinate patterns ===" rg -i "(\[longitude.*latitude\]|\[lat.*long\])" -g "!*.{json,md}" -A 2 echo -e "\n=== Searching for GeoJSON mentions ===" rg -i "geojson" -g "*.{ts,js,tsx,jsx}" -A 2 echo -e "\n=== Looking for documentation about coordinates ===" fd "README|CONTRIBUTING" -e md -X catLength of output: 73164
src/core/focused_geometry/types.ts (1)
38-38
: Verify the impact of optional hash property.Making the hash property optional could lead to inconsistent handling in the codebase. Ensure all consumers of this type properly handle the case when hash is undefined.
✅ Verification successful
The codebase already handles optional hash property safely ✅
The identified usages already implement proper null checking and optional chaining, making them compatible with the optional hash property:
src/features/layers_in_area/atoms/areaLayersLegendsAndSources.ts
: Uses optional chaining and explicit null checksrc/features/layers_in_area/atoms/areaLayersDetailsResource/areaLayersDetailsParamsAtom.ts
: Uses optional chaining🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct usage of the hash property without optional chaining rg -l "geometry\.hash[^?]" --type tsLength of output: 194
Script:
#!/bin/bash # Check the actual implementation in the identified files echo "=== areaLayersLegendsAndSources.ts ===" rg "geometry\.hash" src/features/layers_in_area/atoms/areaLayersLegendsAndSources.ts -A 2 -B 2 echo -e "\n=== areaLayersDetailsParamsAtom.ts ===" rg "geometry\.hash" src/features/layers_in_area/atoms/areaLayersDetailsResource/areaLayersDetailsParamsAtom.ts -A 2 -B 2Length of output: 776
src/core/localization/translations/en/common.json (2)
5-5
: LGTM!The addition of the "or" translation key follows the existing pattern for simple conjunctions.
464-467
: LGTM! Well-structured location-related messages.The new messages provide clear feedback for:
- Location access status
- Error handling
- Success confirmation with dynamic area name
...features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.module.css
Show resolved
Hide resolved
...features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.module.css
Show resolved
Hide resolved
...features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.module.css
Show resolved
Hide resolved
src/features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.tsx
Show resolved
Hide resolved
src/features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.tsx
Show resolved
Hide resolved
src/features/user_profile/components/SettingsForm/ReferenceAreaInfo/ReferenceAreaInfo.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with comments
https://kontur.fibery.io/Tasks/Task/FE-Add-new-feature-Select-current-location-in-Reference-section-of-Profile-20432
Summary by CodeRabbit
New Features
Localization
Improvements
Bug Fixes