V4 #20
V4 #20
Conversation
…ement enhancements
… calibration checks
…l referenceDistance and verify it remains unchanged after handleDistanceModalCancel() is called
…nitSelect in index.js to include logic that updates all pinned measurement labels. Exported setupEventHandlers: Added setupEventHandlers to the __testables export in index.js to facilitate integration testing. Added Integration Test: Added a new test case to index.scale.test.js:268-301 to verify that pinned measurement labels are correctly updated when the global unit changes. Improved Mocks: Updated the L.divIcon mock in index.scale.test.js:35 to correctly capture and return options for verification in tests
Creating a centralized createDistanceLabelHtml helper function in index.js:754 that handles the generation of distance labels with optional "Pin" or "Delete" buttons. Updating drawReferenceVisualization, updateMeasureLabel, and pinCurrentMeasurement to use this new helper. Replacing the regex-based replacement in pinCurrentMeasurement with a clean re-render of the label using the helper function
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 introduces a significant set of advanced features and polish, completing "Phase 3" of development. It focuses on enhancing user experience through persistent settings, robust scale validation, and more flexible measurement tools. The changes also include comprehensive testing to ensure stability and correctness of the new functionalities. 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 pull request introduces a significant and well-implemented feature for manual scaling and measurement. The changes are extensive, covering UI, core logic, and persistence. The code is well-structured, particularly with the extraction of state machine logic into pure, testable functions. The addition of comprehensive integration and property-based tests is excellent and greatly improves the robustness of the new functionality. I've identified one medium-severity issue related to code duplication that could be refactored for better maintainability. Overall, this is a high-quality contribution.
| state.measureMode.pinned.forEach((item) => { | ||
| if (item.ui.label) { | ||
| const sourceIcon = item.source === 'manual' ? '📏' : '📡'; | ||
| const newHtml = `<div class="distance-label distance-label--measure" style="background:${COLORS.MEASURE};">${sourceIcon} ${formatDistance(item.meters, state.preferredUnit)}</div>`; | ||
| item.ui.label.setIcon(L.divIcon({ | ||
| className: 'measure-label', | ||
| html: newHtml, | ||
| iconAnchor: [0, 0], | ||
| })); | ||
| } | ||
| }); |
There was a problem hiding this comment.
There's a small duplication of logic here for generating the HTML for pinned measurement labels. The createDistanceLabelHtml function was introduced to centralize this logic, but it's not used here. Reusing the helper function will make the code more maintainable and consistent.
state.measureMode.pinned.forEach((item) => {
if (item.ui.label) {
const sourceIcon = item.source === 'manual' ? '📏' : '📡';
item.ui.label.setIcon(L.divIcon({
className: 'measure-label',
html: createDistanceLabelHtml({
meters: item.meters,
color: COLORS.MEASURE,
icon: sourceIcon,
showPin: false,
extraClass: 'distance-label--measure',
}),
iconAnchor: [0, 0],
}));
}
});
No description provided.