Conversation
|
🚧 Release candidate build based on efb1aee (potential merge of 25fff58). Assets can be found in the |
Updates the bounding box used for the map where there's no data to show. Something I learned via digidem/comapeo-map-server#2 (comment)
Fixes #470 Now we clear the relevant editing state when leaving the page, whether it's navigating to a different observation or navigating away from the observation details altogether. Probably need to rethink the global editing state setup at some point. Feels pretty fragile and annoying to manage, but not sure if there are better approaches.
Seeing an [error on Sentry](https://awana-digital.sentry.io/issues/7114347194/) that's identical to what's shown in maplibre/maplibre-gl-js#6730 . Upgrades `maplibre-gl-js` to include the fix for the referenced issue.
Addresses some security patches in deps.
Co-authored-by: Andrew Chou <andrewchou@fastmail.com>
- fixes a console error related to unusable react transition group props - displays a loading state for the decline invite button when pressed. usually imperceptible because of how fast it happens but just in case. - updates the pending invite card to visually align with other similar card designs
|
/build-rc |
|
🚧 Release candidate build based on eb3c5dc (potential merge of 821ab98). Assets can be found in the |
Closes #197 Also renames all "lint" commands to "checks".
Just some minor code cleanup.
Not sure if this was a regression caused by `systeminformation` or due to me updating my OS yesterday. Guessing the latter.
Closes #484 . This was a gnarly one. This is what was happening during the leave project flow from a tanstack query perspective: 1. Click confirm button to leave project, which calls the leave project mutation (part of core-react). 2. In core-react when the mutation succeeds, it [invalidates all queries related to projects](https://github.com/digidem/comapeo-core-react/blob/7d148b47230ff31cfb3378d881a111891db7ae58/src/lib/react-query/projects.ts#L337-L339). 3. We then attempt to navigate to the onboarding. 4. In the `beforeLoad` for the onboarding route, we [check to see](https://github.com/digidem/comapeo-desktop/blob/4a6bc583c09e7a0f8ac03a6bc34968cd0f96644f/src/renderer/src/routes/onboarding/route.tsx#L24-L39) if we're already on a project, as we'd rather redirect them to a valid project than to let the user access the onboarding. - We ideally wouldn't need to do this and instead would redirect to some project selection page, but that's a separate concern. The problem was that in (4), we use [queryClient.ensureQueryData](https://github.com/digidem/comapeo-desktop/blob/4a6bc583c09e7a0f8ac03a6bc34968cd0f96644f/src/renderer/src/routes/onboarding/route.tsx#L24) to get the list of projects the user is still a part of. However, this [method](https://tanstack.com/query/latest/docs/reference/QueryClient#queryclientensurequerydata) looks in the query cache for an existing value and only resorts to refetching if it doesn't already exist. In our case a cached value already exists but it's the value prior to leaving successfully, meaning that we were redirecting back to the project that we just left, hence why nothing seems to happen and things are broken on subsequent attempts to leave. The [`revalidateIfStale` option](https://tanstack.com/query/latest/docs/reference/QueryClient#queryclientensurequerydata) is almost exactly what we want in this situation, but it will still return the cached value and do the refetch in the background, which doesn't solve our issue in this case. The simplest solution is to populate the query cache with the updated projects state after leaving succeeds, but before we attempt to navigate to the onboarding as in (3). This way the `ensureQueryData` call in (4) is working with an updated cache and no longer has access to the project that was just left. I explore a different path where instead of using `queryClient.ensureQueryData` in the `beforeLoad`, we use `queryClient.fetchQuery`, which seems more idiomatic and safer. However, noticed some unfortunate effects related to route loading that made the app feel worse. Think it's something that should be revisited later. Ideally, we avoid using stale query values for everything that's used for determining a redirect in a route's `beforeLoad`.
Also updates the random category selection logic to be more similar to actual app usage, where there's a defined set of categories to use based on the type of data being created (observation vs track).
Fixes a regression introduced by #485 . Repro steps: 1. Invite mobile device from desktop 2. On mobile device, accept invite. 3. On desktop, observe the following: <img width="600" alt="image" src="https://github.com/user-attachments/assets/df79646c-f8ff-4992-b092-a33ee8b2bbbc" /> Still a little miffed at the behavior, as it seems to be some sort of race condition or a misuse of router/query. Haven't really wrapped my head around where the actual issue lies, but guessing it's on our end.
When some generic route error occurs, we typically display a panel containing information about the error and a button that reloads the page. However, there are cases where you can get stuck on the page due to UI elements being intentionally disabled. Now we provide a "restart" button which doesn't restart the application entirely, but just loads the "index" page as opposed to the current page, which is usually sufficient. Might be worth using different wording eventually, especially if we introduce something that actually restarts the application.
Using the `file://` protocol in Electron is [generally recommended against](https://www.electronjs.org/docs/latest/tutorial/security#18-avoid-usage-of-the-file-protocol-and-prefer-usage-of-custom-protocols) and it's preferable to use custom protocols for serving app pages. To add to that, I believe that using a custom protocol solves [issues with later versions of tanstack router](https://github.com/digidem/comapeo-desktop/actions/runs/20881769030). Poking around locally, there's something about their URL reload behavior that changed such that it actually triggers a proper reload, which causes errors in the app to surface and breaks things. Using the custom protocol actually aligns a bit better with standard web navigation and there are a couple of nice benefits, such as: - (probably) fewer issues moving forward with tanstack router and doing full page reloads. so many hours fighting against it and over-relying on internal details in recent months... - in the future, this will allow us to do deeplinking and inteception of URLs that use the custom protocol (`comapeo://`) I'm still unsure about the following: - Does Sentry still generally work as expected? I see lots of new logs related to tracing headers or something now, although they seem harmless. - Is the integration code of the custom protocol generally correct? Also worth noting that adding this introduced additional CORS errors relating to maps. Instead of updating/adding more patches, decided to use a fastify hook to add the appropriate header for the relevant responses, which is a bit hacky but more holistic and easier to maintain.
Couple of notable changes: - [v1.146.2](https://github.com/TanStack/router/releases/tag/v1.146.2) introduces some additional redirect safeguarding in router's core ([PR](TanStack/router#6337)) that causes issues when using the `reloadDocument: true` when navigating, which we do in a few places to reset the history. In order to bypass this we patch router-core to allow redirects to URLs that use our app's custom `comapeo:` protocol. Any security/access control measures are thus handled in our app within the protocol handler that's set up in the main process. - Uses `RouteApi.redirect()` instead of the standalone `redirect()` function, which is a little more ergonomic and shouldn't have any user-facing effects ([docs](https://tanstack.com/router/latest/docs/framework/react/api/router/redirectFunction#using-routeredirect-file-based-routes))
Notable changes: - Updates Electron to [v40](https://www.electronjs.org/blog/electron-40-0) - Updates development node version to v24.11.1 ([migration notes from v22 to v24](https://nodejs.org/en/blog/migrations/v22-to-v24))
Co-authored-by: Andrew Chou <andrewchou@fastmail.com>
|
/build-rc |
|
🚧 Release candidate build based on b100152 (potential merge of 850da3b). Assets can be found in the |
Lays the groundwork for being able to run e2e tests in isolation from one another, which enables the following: 1. Being able to run the tests in parallel. Before, there was a specific sequence that needed to occur between tests because of the use of outputs between test projects. Now each test runs with its own data directory so there's no sharing of outputs. 2. Being able to "parameterize" tests such that we can test various scenarios for a given flow. The old test setup was pretty limited in that we were essentially testing a very specific flow (e.g. single player project creator) and if we wanted to test a different flow, it would require a pretty hard to manage implementation. The isolation allows us to write tests that set up their own unique environment/situation. Notable implementation details: - All e2e tests are run in parallel. When running locally, we allow Playwright to attempt to use as many CPU cores as are available. In CI we limit it to 3 max, which is based on the [specs of the GitHub Actions runners](https://docs.github.com/en/actions/how-tos/write-workflows/choose-where-workflows-run/choose-the-runner-for-a-job#standard-github-hosted-runners-for-public-repositories). - Each test runs an instance of the application that uses its own user data directory, which provides data isolation.
Notable changes: - Updates preferred development npm version to 11.8.0 - Replaces usage of `dotenv` with [`loadEnvFile`](https://nodejs.org/docs/latest-v24.x/api/process.html#processloadenvfilepath) (built into Node)
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Not sure why I went with `break-all` when initially implementing, but `break-word` seems more appropriate and is consistent with what we do elsewhere.
Adds the following tests: - Initial state of main panel after project creation onboarding - Project info popup
Only covers the scenario of no other existing members and no invitable devices.
|
🚀 A draft release pointing to 52f0aa3 has been created at https://github.com/digidem/comapeo-desktop/releases/tag/untagged-328f8138929c93afcfb1 |
This PR tracks QA of the release candidate for v1.0.
🍒 All changes to this branch should be cherry-picked from
main.🧪 Create a new Release Candidate build by commenting on this PR with the
/build-rccommand.🚧 An initial Release Candidate build will be triggered shortly, and the build URL will be posted here.
🚀 Merging this PR will trigger a release build.