Feat/mobile ux #6
Conversation
…ve state management
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 comprehensive update to the Snap2Map application, primarily focusing on a more polished and user-friendly experience for mobile users. The changes streamline the process of importing map photos and calibrating them with real-world locations, while also making GPS tracking more intelligent by centering the map only on the initial location fix. The visual design has been modernized with a dark theme and improved accessibility, ensuring a consistent and intuitive interface across the application. Highlights
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-executed overhaul of the UI/UX, particularly for mobile devices. The new layout in index.html is a major improvement, making the app feel more modern and structured. The logic for centering the map only on the first GPS fix is a great UX enhancement, and the implementation using photoPendingCenter and osmPendingCenter flags is clear and effective. The related test updates are also well-written and cover the new behavior. I have one minor suggestion to improve code clarity by removing a redundant line of code. Overall, this is a high-quality contribution.
| }); | ||
|
|
||
| state.osmLocateControl = locateControl.addTo(state.osmMap); | ||
| state.osmPendingCenter = true; |
There was a problem hiding this comment.
The flag state.osmPendingCenter is set to true on this line and again on line 825. This is redundant. Setting the flag only once, immediately before the action that depends on it (i.e., on line 825 before state.osmLocateControl.start()), makes the code's intent clearer and avoids duplication. Please remove this line.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| } | ||
|
|
||
| updateGpsStatus('Waiting for location fix…', false); | ||
| state.photoPendingCenter = true; | ||
| state.osmPendingCenter = true; |
There was a problem hiding this comment.
[P1] Reset pending-center flags when geolocation watch is already active
The new first-fix behaviour depends on photoPendingCenter/osmPendingCenter being set to true before the next GPS update. Those flags are only set inside startGeolocationWatch, but they are executed after an early return when state.geoWatchId is already running. If a user replaces the map photo or recalibrates while the original watch is still active (which loadPhotoMap leaves running), the subsequent call to startGeolocationWatch immediately exits and the flags remain false. The existing watch keeps delivering positions but the new map never recenters on the first fix, which is a regression from the previous implementation that always re-centered. Consider resetting the pending-center flags (or restarting the watch) even when state.geoWatchId is non-null so a fresh map/calibration recenters correctly.
Useful? React with 👍 / 👎.
This pull request introduces a significant UI and UX overhaul for the Snap2Map app, focusing on improving the photo/map import flow, live GPS centering behavior, and visual polish across the interface. The changes modernize the layout, clarify user actions, and refine the integration of geolocation with both the photo and OSM map views. Additionally, the codebase is updated to better manage UI state and interactions, and tests are adjusted to reflect new behaviors.
UI/UX Improvements:
index.html: The map photo import is now presented as a dedicated step with improved instructions, a clearer file picker experience for mobile, and a visible "Replace photo" button after import. The reference pair table and calibration status are visually upgraded for clarity and accessibility.renderPairList: Pair rows now use dark theme colors, with improved indicator and button styling for better contrast and feedback.Geolocation and Map Centering Enhancements:
photoPendingCenterandosmPendingCenterflags in state, with supporting logic inupdateLivePosition,startGeolocationWatch, and related functions. [1] [2] [3] [4] [5] [6] [7] [8] [9]Photo Import and UI State Management:
setPhotoImportStateto toggle visibility of the photo placeholder and replace button, ensuring the UI reflects whether a photo is loaded. This is called on initialization and after photo import. [1] [2] [3] [4] [5] [6]Visual and Accessibility Refinements:
Test Adjustments:
src/index.locate.test.jsto reflect the new locate control configuration (setView: false) and to track map centering calls before and after location events. [1] [2]