Feat/instant usage v4 #26
Conversation
… and its implementation plan
…tionality for setting reference distances and measuring
…ion and interaction improvements
…s for centroid calculation
…ith weight influence
…tric properties and edge cases
…r improved accuracy
… null for coincident pixel points
…e over GPS calibration
… and unit selection
…te for reference distance feature
…ng reference distances
…ne module with comprehensive tests
…th refactored state management
…istance input modal for reference distance entry with validation and unit selection
…es for scale and measure markers and labels in the UI
…into a reusable function for cleaner code
…d rely on conversion function for improved clarity
…ility and integrate into distance validation
…lined user experience
… position and enhance user feedback for missing scale.
…nd implement one-tap calibration functionality
…ases for improved clarity and consistency
…ale is set and no pairs exist
…g for prompt classes
…dPhotoMap function; update related test case
…ion; update related test case for clarity
…est passes by validating state
… ensure logic path is verified by state conditions
…calibrateMap to utilize it
…n and active calibration states
…ode organization and readability
…ases of implementation and testing
…add default rotation setting and refine prompt visibility logic
….md:100-112 to match the correct implementation in the source code. Tests: I improved the test case in scale.test.js:379-385 to specifically assert that 0.6090m (which rounds to 12 inches) correctly results in 2' 0".
…to reflect the correct implementation of getMetersPerPixelFromCalibration. The documentation now correctly states that the calibration maps pixels directly to ENU coordinates (meters), eliminating the need for degree-to-meter conversions. Removed Outdated Constant: I removed the unused METERS_PER_DEGREE_EQUATOR constant from scale.js:8, as it was part of the outdated logic and no longer needed. Verified Implementation: I confirmed that the actual implementation in scale.js already correctly handled the scale extraction without the unnecessary conversion. Fixed Lint Errors: I resolved a lint error in index.scale.test.js that was uncovered during the quality check process.
…fy reference scale and initial suggestion prompts
…ementation in transformations.js, which returns a structured object with scale, rotation, and translation instead of a raw matrix. Calibration Layer: Updated the calibrateMap snippet to include the calibrate1Point helper function as it exists in calibrator.js, ensuring the return structure (including status, metrics, and quality) is consistent with the actual API
Extracting the hiding and showing logic into two new helper functions: hideInstantUsagePrompts() and showInstantUsagePrompts(). Replacing all manual manipulations of dom.instantUsagePrompts.classList with calls to these helpers. Simplifying setPhotoImportState and beginPairMode by using the new hideInstantUsagePrompts() helper.
…convert rotation degrees (0, 90, 180, 270) into their corresponding labels ("North-up", "East-up", etc.).
Updated handleDistanceModalConfirm: Modified the toast message in index.js:1042 to dynamically include the selected rotation label when a 1-point calibration becomes active after setting the scale.
Updated handleOneTapClick: Modified the toast message in index.js:1137 to dynamically include the selected rotation label when a 1-point calibration is active after pinning a location
Added case 'ft-in': to the switch statement in convertToMeters. Updated the JSDoc to include 'ft-in' as a valid unit. Both 'ft' and 'ft-in' now correctly convert decimal feet to meters using the METERS_TO_FEET constant. Updated scale.test.js:335: Added a new test case converts feet-inches (decimal feet) to meters to ensure this unit is handled correctly in the future
…d case. Ensures state.osmGeoPrompted is correctly set even if permission was previously given. Added regression test in src/index
Summary of ChangesHello @cs-util, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a major enhancement to the application's usability by introducing 'Instant Usage' capabilities. Users can now gain immediate value from imported maps, performing measurements and even tracking live positions with minimal setup. The changes span mathematical transformations, calibration algorithms, and a significantly improved user interface, all built on a more modular and thoroughly tested codebase. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive pull request that introduces significant and valuable new features, "Instant Usage" and "Reference Distances". The implementation is of very high quality, demonstrating a thoughtful approach to architecture and user experience. The refactoring of index.js into more modular, testable components (scale.js, scale-mode.js) is a fantastic improvement for maintainability. The test coverage is exceptionally thorough, including unit, integration, and property-based tests, which provides great confidence in the correctness of the new logic. The feature specification documents are also very detailed and helpful. I have only a couple of minor suggestions for improving the documentation.
docs/feat-instant-usage.md
Outdated
|
|
||
| ## 4. Edge Cases & Considerations | ||
| * **Unstable 2-Point Fit**: If two GPS points are extremely close together, the rotation becomes numerically unstable. In this case, the system should either warn the user or offer to stick to the "North-up" assumption. | ||
| * **Missing Scale**: If the user provides 1 GPS point but hasn't set a scale yet, the "Live" mode remains disabled until the scale is defined (either because the user later decided to set the reference scale or because he decided to set a second gps reference point). |
There was a problem hiding this comment.
For better clarity and more inclusive language, consider rephrasing this sentence. Using 'they' instead of 'he' is more neutral, and the sentence could be made more concise.
| * **Missing Scale**: If the user provides 1 GPS point but hasn't set a scale yet, the "Live" mode remains disabled until the scale is defined (either because the user later decided to set the reference scale or because he decided to set a second gps reference point). | |
| * **Missing Scale**: If a user provides one GPS point without having set a scale, "Live" mode remains disabled. It will be enabled once a scale is defined, either by setting a manual reference scale or by adding a second GPS reference point. |
| ```javascript | ||
| const METERS_TO_FEET = 3.28084; | ||
|
|
||
| function formatDistance(meters, unit) { | ||
| switch (unit) { | ||
| case 'ft': return `${(meters * METERS_TO_FEET).toFixed(2)} ft`; | ||
| case 'ft-in': { | ||
| const totalInches = meters * METERS_TO_FEET * 12; | ||
| const feet = Math.floor(totalInches / 12); | ||
| const inches = Math.round(totalInches % 12); | ||
| if (inches === 12) { | ||
| return `${feet + 1}' 0"`; | ||
| } | ||
| return `${feet}' ${inches}"`; | ||
| } | ||
| default: return `${meters.toFixed(2)} m`; | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The code snippet for formatDistance appears to be out of sync with the implementation note on line 318, which mentions support for cm and mm. The actual implementation in src/scale/scale.js also includes this logic. It would be beneficial to update this snippet to reflect the full functionality, ensuring the documentation is consistent with the implementation.
… in the Instant Usage feature documentation.
… the implementation
No description provided.