Repository navigation
feat(mobile): photos attached to features, through the camera on the platform trait (#1902) - #2025
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. Walkthrough
Merge Risk:
 |
Member
Author
|
On the Android 15 emulator (x86_64), with its own camera app (Camera2, showing the emulator's virtual scene).
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Code reviewBugs
Security
Performance
Quality
AGENTS.md
No earlier review threads existed, so none were left alone as already raised. Inline comments posted: 3 ( Change sizeCompared with
|
… subtype-filled fields aren't asked for, typing ends when the keyboard goes
# Conflicts: # crates/ui-touch/src/lib.rs
…ther selections as they were
# Conflicts: # crates/ui-touch/src/lib.rs
…l says what it left out, the track isn't copied each frame
# Conflicts: # crates/ui-touch/src/lib.rs
…st goes at once past the limit
# Conflicts: # crates/ui-touch/src/lib.rs
…text is said, a photo survives the app ended under the camera
Contributor
Code reviewBugs
Security
Performance
Quality
AGENTS.md
Not verified: I didn't build or run the tests. The no-Bash setup limited checks to reading the staged diff and source files. Change sizeCompared with feat/mobile-background-track (merge base 8caa67e to PR head 60db964).
|
18 of 26 tasks
…d whole-number codes match exactly
# Conflicts: # crates/ui-touch/src/lib.rs
# Conflicts: # apps/rustgis-mobile/README.md # crates/ui-touch/src/lib.rs
- A track that can't be added (editing or the layer refused it) is kept being recorded and said, so Stop tries again instead of losing its positions; the desktop interface isn't switched to while it is kept; tested. - Positions dropped past FIXES_KEPT are counted and said when a track or an average missed them; tested. - The More sheet is sized for the track's row while a track is recorded with positioning off. - Typing in a sheet the keyboard leaves short (a phone held sideways), the rows above the capture fields step aside until the keyboard goes, so the field stays in sight.
- TrackService's refusal reaches the activity at once: it stops keeping positions out of sight then, and stops asking for them if already out of sight, not at the next frame. - The README says Android ending the activity ends a track too.
# Conflicts: # crates/ui-egui/src/lib.rs
- Adding a feature with photos names the field they went in, the layer's own maybe; attaching names it too; tested. - A camera that wrote a photo and then said it was cancelled keeps the photo, unattached, and that is said, instead of the photo being deleted.
# Conflicts: # apps/rustgis-mobile/java/org/rustgis/mobile/MainActivity.java # crates/ui-egui/src/lib.rs # crates/ui-touch/src/lib.rs
Contributor
Code reviewI reviewed the full diff against the base branch, the staged AGENTS.md, and the surrounding source. I could only read files, not build or run tests. I did not repeat the eight earlier threads, which are all resolved and whose fixes are in the diff. I checked each fix against the code: held text kept as typed, the camera result counted as awaited on restore, the photo kept in Bugs
Security
Performance
Quality
AGENTS.md
Not verified
Change sizeCompared with the base branch: the whole PR against
|
- A track with breaks where the device's position was off is stored in parts, one for each run, not joined across the gap; the overlay breaks there too, and the parts and any position alone between breaks are said; tested. - A position older than the last kept is left out and counted, so the line doesn't double back; tested. - The track overlay starts a new run where consecutive positions are across the antimeridian. - `finish` says in its doc comment why it runs on the window's thread. - The kept-track test checks that the reason the layer refused it is said before that it is kept.
- A track whose positions are each alone between breaks says that is why it wasn't added, and how many positions were left out, instead of "fewer than two positions"; tested.
…ile-background-track
…d-track # Conflicts: # apps/rustgis-mobile/README.md # crates/ui-egui/src/lib.rs # crates/ui-touch/src/lib.rs
- FIXES_KEPT is now 200,000, as many positions as the activity keeps out of sight (MOST_EVENTS), so a two-day batch handed over on coming back goes whole into the track, as the README says.
# Conflicts: # apps/rustgis-mobile/java/org/rustgis/mobile/MainActivity.java
Contributor
Code reviewI checked the diff against the base, the Rust touch UI and engine call sites, and the Android Java lifecycle. The prior threads cover most of the design, and I didn't re-raise them. Two new inline comments are posted (below). The rest of the new code looks correct: the Platform trait change compiles through every construction site, Bugs
Security
Performance
Quality
AGENTS.md
Already raised (not repeated)
Change sizeCompared with
I did not run the build or the tests; I reviewed by reading the code. |
# Conflicts: # apps/rustgis-mobile/AndroidManifest.xml # apps/rustgis-mobile/README.md # apps/rustgis-mobile/java/org/rustgis/mobile/MainActivity.java # apps/rustgis-mobile/src/phone.rs # crates/engine/src/platform.rs # crates/ui-egui/src/lib.rs # crates/ui-touch/src/lib.rs
- A photos field of that name that isn't text says nothing was added or attached, instead of reading as if the feature went in without its photos. - Only the photos a field doesn't list already are counted in the notices, and attaching none new makes no edit; tested. - A photos field made for a feature that is then refused is taken away again, and said, so a refused add leaves the layer's design as it was; tested. - A held list ending in `;` gets no empty entry before the new photo; tested.
Member
Author
|
@coderabbitai approve |
| if (file == null && result == RESULT_OK) { | ||
| add("failed\tThe camera app took a photo, but RustGIS lost track of it: look in its photos folder."); | ||
| } else { | ||
| add(result == RESULT_OK ? "failed\tThe camera app said it took a photo, but wrote none." : "cancelled"); |
Contributor
There was a problem hiding this comment.
Misleading notice when the camera wrote its own copy (confidence ~45%). This branch runs when RESULT_OK comes back with no file or an empty one. The message says the camera "said it took a photo, but wrote none". Some camera apps ignore EXTRA_OUTPUT, save to their own gallery, and return only a thumbnail. The photo then exists, but RustGIS reports that none was written and never says where to look. The PR's own testing notes flag this case as untested.
Suggested direction: when RESULT_OK arrives without our file, check whether the result carries a thumbnail in data and say that the camera may have kept the photo in its own gallery, so it can be found and attached by hand. The current wording is only accurate when the camera really wrote nothing.
Contributor
Code reviewBugs
Security
Performance
Quality
AGENTS.md
Already raised, left alone: 12 review threads are on the PR, all resolved and none outdated, and each is addressed in the current code: the non-text Checks I ran by reading the code: Change sizeCompared with base: the whole PR against main (merge base 4a53308 to PR head 3b2f498).
Largest changes:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
Action performed
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 3 of #1902: photos attached to features, which also completes phase 1's platform trait with the camera. Stacked on #2024.
Platform::take_photo()asks the device's camera;Event::Photo { name, path }(photoline) follows, orCancelled. The front end asks throughServices::take_photo, and the shell puts each photo taken inRustGisApp::photosfor the interface. The web app leaves it unset.ACTION_IMAGE_CAPTUREto the device's camera app with a content address offered by the newPhotoProvider(not exported; the camera app gets leave to write that one address), so the photo lands in the app's photos folder,files/photos/photo-YYYYMMDD-HHMMSS.jpg, and RustGIS needs no camera permission. A photo is reported only when the camera says it took one and wrote it; an empty file left by a cancelled camera is removed. The manifest names the provider and asks to see camera apps (<queries>, Android 11 and later).crates/ui-touch/src/photos.rs): Take Photo over the Capture sheet's fields; each photo shows as a chip, which a tap takes off. The next feature added names its photos in the layer's photos field, a text fieldphotos(orphoto) listing paths separated by;(as a report's attachments field does). A layer without one gets aphotosfield when its first photo is attached, and the sheet says so; the form doesn't offer that field for typing. In Edit mode, Add Photo attaches a photo to the picked feature after the ones it has, as one edit that undoes.Not in this pull request
Testing
cargo test -p rustgis-engine --lib platform: aphotoline read, and one without a path refused.cargo test -p rustgis-ui-touch(25 tests): photo paths join once each; a photo arriving while the sheet is up waits for the next feature; adding it makes the layer's photos field, names the photo and says so; in Edit mode a photo arriving goes to the picked feature after the first, and Undo takes it off again.-D warnings, which caught the web app'sServices),cargo xtask layers, the wasm build and Android clippy pass. The full test suite was not run locally.photo-20261114-020043.jpgchip, and Add Point added Well B with "Added a photos field to Wells"; in Edit mode on the plot, Add Photo, shutter and check attached a photo ("Attached 1 photo(s) to the feature", with the Plots photos field made).Related issue
Part of #1902.
Summary by CodeRabbit