Conversation
|
| Status | Count |
|---|---|
| 22 | |
| 10 | |
| 1 |
Click to toggle table visibility
| Name | Status | Previous | Current |
|---|---|---|---|
@comapeo/cloud |
0.3.0 | 0.4.0 | |
@comapeo/core-react |
10.0.1 | 11.0.4 | |
@comapeo/core |
6.0.2 | 7.0.1 | |
@comapeo/ipc |
7.0.0 | 8.0.0 | |
@comapeo/map-server |
1.0.1 | 1.1.3 | |
@gmaclennan/zip-reader |
- | 1.0.0 | |
@hyperswarm/secret-stream |
6.8.1 | 6.9.1 | |
@mapbox/mapbox-gl-style-spec |
- | 14.21.0 | |
@mapbox/point-geometry |
- | 1.1.0 | |
@sqlite.org/sqlite-wasm |
- | 3.51.2-build8 | |
@turf/bbox-polygon |
- | 7.3.4 | |
@turf/boolean-point-in-polygon |
6.5.0 | 7.3.4 | |
@turf/clone |
- | 7.3.4 | |
@turf/point-to-line-distance |
- | 7.3.4 | |
@turf/point-to-polygon-distance |
- | 7.3.4 | |
@turf/polygon-to-line |
- | 7.3.4 | |
@turf/projection |
- | 7.3.4 | |
@turf/rhumb-bearing |
- | 7.3.4 | |
@turf/rhumb-distance |
- | 7.3.4 | |
bare-ansi-escapes |
- | 2.2.3 | |
bare-assert |
- | 1.2.0 | |
bare-inspect |
- | 3.1.4 | |
bare-type |
- | 1.1.0 | |
csscolorparser |
- | 1.0.3 | |
noise-handshake |
4.1.0 | 4.2.0 | |
point-in-polygon-hao |
- | 1.2.4 | |
ready-resource |
1.1.2 | 1.2.0 | |
robust-predicates |
- | 3.0.3 | |
smp-noto-glyphs |
- | 1.0.0-pre.0 | |
styled-map-package-api |
- | 5.0.0-pre.4 | |
typebox |
1.1.10 | 1.1.16 | |
wsl-utils |
0.1.0 | - | |
zip-writer |
- | 2.2.0 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| earlyAccessStore, | ||
| appUsageStatsStore, | ||
| }: AppProvidersProps) => { | ||
| const mapServerListenPromise = React.useMemo( |
There was a problem hiding this comment.
I had to memoize this because I was getting an error:
"Listen method has been called more than once without closing"
- Happened on force close and restart
Because the appRpc.mapServer.listen() call was happening on every render of the AppProviders component.
It was also complaining about needing an argument (hence the empty object)
There was a problem hiding this comment.
Good point, I'll have a think about improving the ergonomics of the api for this - it's too easy right now to use it in a way that causes errors.
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
…tendfor package json tests.
| )) { | ||
| const backendVersion = backendMapeoDependencies[dependency]; | ||
| if (!backendVersion) continue; | ||
| // For file: dependencies, compare just the filename (paths may differ) |
There was a problem hiding this comment.
Do you think it's ok to do this just for tests? That way more pressing, problematic test failures will be visible.
There was a problem hiding this comment.
You could also just comment out the expect during dev until it's ready - we shouldn't be using file dependencies by the time the pr is finished
There was a problem hiding this comment.
Or don't run these tests on draft prs
|
I've update the core-react PR #143 with what I hope is a fix for this issue. Here is an updated tarball with the latest from commit digidem/comapeo-core-react@22f6d95 |
| earlyAccessStore, | ||
| appUsageStatsStore, | ||
| }: AppProvidersProps) => { | ||
| const mapServerListenPromise = React.useMemo( |
There was a problem hiding this comment.
Good point, I'll have a think about improving the ergonomics of the api for this - it's too easy right now to use it in a way that causes errors.
| )) { | ||
| const backendVersion = backendMapeoDependencies[dependency]; | ||
| if (!backendVersion) continue; | ||
| // For file: dependencies, compare just the filename (paths may differ) |
There was a problem hiding this comment.
You could also just comment out the expect during dev until it's ready - we shouldn't be using file dependencies by the time the pr is finished
| )) { | ||
| const backendVersion = backendMapeoDependencies[dependency]; | ||
| if (!backendVersion) continue; | ||
| // For file: dependencies, compare just the filename (paths may differ) |
There was a problem hiding this comment.
Or don't run these tests on draft prs
| {projectId, receiverDeviceId: deviceId, mapId: 'custom'}, | ||
| { | ||
| onSuccess: result => { | ||
| Promise.resolve(result).then(mapShare => { |
There was a problem hiding this comment.
Something up with my code here - I'll take a look
|
@gmaclennan And for the sendMapShare: [Error: fetch failed: [start] Cannot convert 'http://127.0.0.1:45635/mapShares' to a Kotlin type.] The bird thinks: But again, I can't figure anything else out so don't know if that is the actual case |
|
@gmaclennan But then I tried a couple other maps and they worked with no error to get to the next step. I haven't yet gotten further than that, yet. Sorry. demotiles-z2 (1).smp And this one: I am going to continue on to next steps after I clean this up a bit (UI in that background map screen isn't quite right), but I thought you wanted to know about where I was able to get so far. |
|
@ErikSin Sorry for the messy and repeated pushes but I somehow messed up the node_modules/ package lock and had to go back and start from scratch.
|
|
Just starting to review, and just noting that my package lock is slightly different when i run I am using node-v24.13.0 and npm-v11.6.2 |
This is not working as expected Screen.Recording.2026-04-06.at.5.05.14.PM.movIt seems like from one device it doesnt matter if your actively on the project. But for the other device it does matter? I initially thought it was one of the devices was a particant and not a coordinator, but that doesn't effect the outcome. I don't know if it matters that much? but weird behaviour none the less |
ErikSin
left a comment
There was a problem hiding this comment.
Its better, now that we have upgraded core and core react, but its still not working fully as expected. See the videos below, but it seems like my samsung Galaxy A03s is having a lot of troubles sending and recieving the cancel in a timely manner. It also is having trouble finding the other device.
Im not sure what the best way to move forward is. This feature doesn't feel ready to release as is.
| <View style={styles.buttonsContainer}> | ||
| {warningInfo.warning !== 'space' && ( | ||
| <PrimaryButton fullSize text={t(m.accept)} onPress={handleAccept} /> | ||
| )} | ||
| {declineStatus === 'pending' ? ( | ||
| <Loading size={12} /> | ||
| ) : ( | ||
| <SecondaryButton | ||
| fullSize | ||
| text={t(m.decline)} | ||
| onPress={handleDecline} | ||
| /> | ||
| )} | ||
| </View> | ||
| </View> |
There was a problem hiding this comment.
This UI is a little off.
- for buttons we use a
<UIActivtyIndicator/>, not<Loading/> - we need block both button if were going to show a loader, not just the decline button
| ); | ||
| }; | ||
|
|
||
| const handleDecline = () => { |
There was a problem hiding this comment.
This is taking more than 2 minutes to register on the other phone:
Screen.Recording.2026-04-06.at.5.31.50.PM.mov
| ? Math.floor((currentTime.getTime() - mapShare.mapShareCreatedAt) / 1000) | ||
| : 0; | ||
|
|
||
| const cancelShare = React.useCallback(() => { |
There was a problem hiding this comment.
This is taking several minutes for the other device to register:
Screen.Recording.2026-04-06.at.5.39.06.PM.mov
| } | ||
| }, [mapShare, navigation]); | ||
|
|
||
| const handleCancel = React.useCallback(() => { |
There was a problem hiding this comment.
There is some really weird behaviour. The progress bar of the receiving phone seems to be quite far behind. And once the sending phone has completed, I cannot press the cancel button on the receiver phone. In this video i am trying multiple times to press "cancel" and I cannot:
Screen.Recording.2026-04-06.at.5.47.18.PM.mov
yes actually there was a change! So sorry I have no idea why! Adds the bolded line. |
Can you push these changes? |
…ges in the same screens.
ErikSin
left a comment
There was a problem hiding this comment.
I tested it with an APK, and things do seem to be working better...Im still not convinced that its in a state to release. But i am going to send an APK to QA and let them decide whether the frontend should pause on development (and wait for the backend team to update), or if we can go ahead with merging and releasing as is
| { | ||
| onSuccess: () => { | ||
| navigation.goBack(); | ||
| }, |
There was a problem hiding this comment.
Now that the state is being handled by the parent, you need to get rid of this goBack() i believe
There was a problem hiding this comment.
If you leave take this out we just stay on the Waiting for device to Accept Map screen and don't go back when you hit cancel. So, still need to have that navigation go back here.
There was a problem hiding this comment.
Im getting a flash of the waiting For Accept screen though. I thought it was due to this, but maybe its something else. Ill take a video
There was a problem hiding this comment.
Screen.Recording.2026-04-07.at.3.25.40.PM.mov
There was a problem hiding this comment.
Oh ok. I think it was the UI falling through. The status wasn't any of the above/ specified, so the main UI was showing, which is the waiting screen. But now that I have added the pending status, that will show the loader instead (at least that is what I am seeing).
| const {shareId} = route.params; | ||
|
|
||
| const mapShare = useSingleSentMapShare({shareId}); | ||
| const {mutate: cancelMapShare} = useCancelSentMapShare(); |
There was a problem hiding this comment.
I think we need catch the status==='pending' here, can show a loading state when that is true
…atus for showing a loading state. Makes sure to navigate back instead of to background maps.
Should fix issues with delays on some actions on certain phones.
|
Just to mention again, the flashing of screens and the need to catch different states that @ErikSin is catching in the review are mostly related to the architectural decision to split this into screens, which I pointed out in #1704 (comment) and in several conversations before that. It is possible to keep finding these edge-cases and fixing them as you have been, but changing the architecture would simplify this a lot and avoid these bugs appearing in the first place. |
I changed it to the way you wanted it which is what was actually causing that flash that Erik mentioned in I left out the one screen that is arrived at through user interaction but combined all of the restl. |
closes #1570
closes #1757
closes #1755
closes #1756
closes #1784
Adds the Map Sharing Feature.