DigitalCommons / mykomap

A web application for mapping initiatives in the Solidarity Economy
3 stars 0 forks source link

[CWM] First iteration MM with MapLibre #263

Closed lin-d-hop closed 1 week ago

lin-d-hop commented 1 month ago

Track under CWM project in clockify

Description

The first deliverable of the new MM incarnation. This is the most basic creation using the new technologies:

  1. Create a clean app with MapLibre, without react binding
  2. Generate GeoJSON data on the fly using just random IDs and lng-lats.
  3. Choose appropriate Spiderify plugin (e.g. https://github.com/ohrie/maplibre-spiderfier https://github.com/nazka/map-gl-js-spiderfy )

Optimising

Its likely that you'll want to pick off low-hanging optimisations fruits as you go. Please don't go too deep into the rabbit hole yet. We want to balance between having a fast map, and ensuring that we tick off the big tasks before we get too deep into the details. Can we hold each other to account on this? Think 80:20 rule - if you are seeking the last 20% of optimisations then let's stop and get back to the 80% tasks.

Working Practice

This issue is to be worked on by @rogup and @wu-lee, pairing. Perhaps it would be helpful to set some practices for pair work? Such as checking in on public channels, regular commits, committing to being transparent on what you are working on. WDYT?

Acceptance Criteria

More Context

Overview doc Timeline Planning doc Will Glitchtip Work with MykoMaps #250

Estimates:

rogup commented 1 month ago

Have done a bit of searching and looks like this is the most common library used for internationalisation https://www.npmjs.com/package/react-i18next

The syntax looks quite simple, and also allows for easy runtime language switching, which would be useful if we want to support this in the future to match the site that the map is embedded in.

The translated strings are supplied in a standard JSON, which can be integrated into a pipeline that uses external translation services

rogup commented 1 month ago

I'm going to try creating the app using Vite as recommended here https://react-redux.js.org/introduction/getting-started#create-a-react-redux-app

It looks like it performs faster in development when making changes. Currently MM uses Webpack, and rebuilding the code after making changes to see the effect takes a while.

I'm not sure how/if Vite will work with our config setup, where we build the app by combining 2 codebases. But let's try and we can always change to Webpack if it doesn't work. I think we should try to find a way for development to be more responsive as part of all these changes.

rogup commented 1 month ago

So I've created a repo for the new React project here https://github.com/DigitalCommons/mykomap-react and added basic map functionality listed in the description of this ticket (mostly copied over from previous demos). The app isn't using any Redux functionality yet, but I expect this will be useful later when we add search/filter.

I selected this Spiderify plugin https://github.com/nazka/map-gl-js-spiderfy since it is actively maintained, the code is quite nice, it works with canvas rendering, and it seemed easy to make changes so that it works with our use case. The fork is here, and hopefully we'll be allowed to eventually merge it back into the source repo, since our changes are quite useful. https://github.com/DigitalCommons/map-gl-js-spiderfy/tree/improvements-for-mykomap

I think we should maybe reduce the scope of this issue, and add time onto later tickets, because it doesn't make sense to spend time on some of these tasks right now:

For this ticket, I think next steps are:

I've used 2 days of time so far

lin-d-hop commented 1 month ago

Thanks @rogup. Glad to see you are making progress.

WRT reducing scope:

Hope that helps :)

If sorting out hook-runner on dev2 is useful now let's go for it.

lin-d-hop commented 4 weeks ago

Meeting notes 15/8 @ms0ur1s and @wu-lee to have a look at using Github pages rather than hook-runner at this stage. @rogup and @wu-lee to have a whiteboarding conversation to create a plan for the remainder of this issue, design the solution and get to a space in which both can work on the issue

rogup commented 4 weeks ago

We also want to do a test with a very basic filter to see if MapLibre's in-built clustering is fast enough during re-indexing.

rogup commented 4 weeks ago

@wu-lee @ms0ur1s Here are some of the ideas we came up with for working collaboratively:

How do these sound and do you have any other ideas @ms0ur1s ?

wu-lee commented 3 weeks ago

@lin-d-hop when you say:

can we implement with current translations standards?

I'm not sure if you mean "current industry translation standards" (meaning something like i18next) or "existing DCC translation standards" (meaning the stuff which Mykomap currently uses, i.e. vocab.json files)

Maybe we can have both, maybe we don't need the latter (although we'll be discarding a bunch of existing DCC standards if we don't), but just want some clarification for the scope of this issue?

The current vocab.json files actually work reasonably well, and they weren't something I was intending to discard, at least not without some concrete reason to. If we do, we probably need some clarity to what the re-implementation costs might be for the data-factory side, which is currently designed to work with them and more generally SKOS vocabs.

lin-d-hop commented 3 weeks ago

I'm not sure if you mean "current industry translation standards" (meaning something like i18next) or "existing DCC translation standards" (meaning the stuff which Mykomap currently uses, i.e. vocab.json files)

I mean the way MM currently does translations. If we can get away with what we are currently doing and it works well enough I would say let's keep it and minimise further scope creep.

wu-lee commented 3 weeks ago

Just noticed there's another scope question:

@lin-d-hop wrote:

Create a clean app with MapLibre, without react binding

Whereas the above project mykomap-react is react-based.

This poses a dilemma when integrating Glitchtip, one of the things @rogup and I agreed I might do next, as when creating a new project for that, as there are separate options for "JavaScript" and "React". Continue with React or scale back to pure MapLibre?

Posting this for discussion; I shall for now add a React project for use with mykomap-react, I suspect adding another one for mykomap and a pure MapLibre iteration will not be that much more work, and at the moment we're still just checking out Glitchtip.

wu-lee commented 3 weeks ago

Ok, after whacking a lot of moles, I have now managed to get the mykomap-react application to post an error on Glitchtip.

image

The Glitchtip DSN is created via a key stored in the project .env file called VITE_GLITCHTIP_KEY. There's another key I needed to add to that, VITE_MAPTILER_API, we'll need to have a shared .env file or some other means of storing this safely outside of version control. And all the keys have to have VITE_ prefixed for them to appear in the import.meta.env object. And Glitchtip needs to be whitelisted in my uBlock Origin plugin for the Sentry client to be allowed to post to it. And there were various other problems with putting breakpoints in code and weird errors from the arse end of sentry resulting from the above, plus maybe others, some of which seem to require a restart of the vite runner via npm run start .

But after addressing all these, it worked. There is a project I've created called "mykomap-react", accessible by a new group "developers" which was required to exist for me to create the project.

Note this demonstrates error reporting, not performance or tracing, which is what we'll need in the first instance. I think there is an API feature for "performance" in Glitchtip/Sentry, however, but will investigate that next/later.

lin-d-hop commented 3 weeks ago

Thanks @wu-lee

Continue with React or scale back to pure MapLibre?

If the project has moved to be fully react based and the whole dev team agrees then I'm happy and will write issues accordingly.

wu-lee commented 3 weeks ago

The mykomap-react project is mostly the output of one of the standard react templates at the moment I think... plus some MapLibre code bits, but not the react integration thereof. I will need to play around a bit to get a feel about the possibilities.

ms0ur1s commented 3 weeks ago

@wu-lee @ms0ur1s Here are some of the ideas we came up with for working collaboratively:

* 15min standups every day that more than 1 of us are working on the same day. Preferably in the morning at 10am.

* Separate days into 3 blocks on the spreadsheet rather than 2 (for me, roughly 9:30-12:00, 1:00-3:30, 4:00-6:30 but might be slightly different for others)

* Do whiteboard sessions with at least 1 other dev and any important stakeholders before starting a new piece of work, where we get clear on the specification, high-level design, and work plan. @lin-d-hop maybe for each of the tasks from the high-level planning doc, the first ticket could be to hold a whiteboard session with the outputs being the ticket(s) for the actual dev work?

* Try trunk-based development https://www.atlassian.com/continuous-delivery/continuous-integration/trunk-based-development, which is basically

  * one single trunk branch ('main') which we keep in a working, releasable state. CI runs on every commit and deploys to a QA build.
  * regular commits (at least 1 per day)
  * commit directly to trunk for very simple changes, or have short-lived PRs that are merged in <2 days for changes that need code review
  * this may involve having 'release toggles' in the code config for large features that require us to gradually push bits of the feature to the trunk that need to be "turned off", if they would break the build whilst in their incomplete state https://martinfowler.com/bliki/FeatureFlag.html

How do these sound and do you have any other ideas @ms0ur1s ?

Sounds really good @rogup.

rogup commented 3 weeks ago

@ms0ur1s Yeah I think for it to work, we need to split down changes into smaller pieces that we don't work on for > 2 days (which might be longer than 2 days in absolute terms since we work part-time and some things need to be done async). But I think this is good practice in any case, for a codebase that we're all going to be committing to frequently.

And yes, we'll need to add UTs as we go, and setup GitHub workflows to run these on every commit to main

rogup commented 3 weeks ago

@wu-lee I've copied a search bar UI element that I found in an online example and added a filter function that filters over marker name (which are all in the format of Marker <number>. I tried adding some of the state to Redux to see if this works, since we'll need to be doing this later anyway.

The filter function can be found in this file https://github.com/DigitalCommons/mykomap-react/blob/main/src/data/geojson.ts

I've deployed this to GitHub pages here https://digitalcommons.github.io/mykomap-react/ It's good news - the filtering became very fast, once I optimised the function to run in O(n) by ensuring the IDs were already sorted, and the clustering seems to be instant! MapLibre must have really optimised SuperCluster. And since we're just using references to the original data in our filtered array, there doesn't seem to be much delay due to GC.

rogup commented 3 weeks ago

@ms0ur1s One last thing for this ticket which I think we maybe need to cover is to decide the appearance of markers/clusters/spiders.

Currently we're using a random free marker image that I found online, and clusters are painted in a style that I just copied from an online demo (3 different colours depending on the number of children in the cluster).

Would you be able to spend a bit of time doing a quick mock-up of what markers/clusters/spiders should look like, and finding an asset for the marker icon? (We can maybe reuse the asset from the existing Mykomap repo but I haven't looked into how to do this.) We probably want the UI to appear more similar to the style of current Mykomap, but we have a few more constraints. The clusters are a rendered 'circle' layer type, not an image, so can't have an opacity gradient since this isn't supported by MapLibre. They can have a fixed opacity, fixed fill colour, font colour, and optional border (with a defined width and colour). The full options are here https://maplibre.org/maplibre-style-spec/layers/#circle and if an option supports 'data-driven styling', we can make it depend on the number of children in the cluster, which is a nice feature that we may want to use.

This all probably doesn't matter too much, as long as the UI looks reasonable, so I don't think we should spend more than half a day on this.

rogup commented 1 week ago

Marcel's review notes: https://docs.google.com/document/d/1yJM_Em4kMdBVqmwHrDrXG28gDDEYqSFx4Ao-V5KsrJ4/edit?usp=sharing

rogup commented 1 week ago

The demo using markers generated on the fly is here: https://digitalcommons.github.io/mykomap-react/

But note we now also have a working skeleton of the app using mock data from a backend: https://dev.maps.coop/cwm/?datasetId=test-500000

lin-d-hop commented 1 week ago

This is excellent! Wonderful work! This feels like a solid starting point before any optimisations and vector tiles 🎉