Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MapKnitter Upgrades planning (overall) #300

Open
21 of 59 tasks
jywarren opened this issue Jan 23, 2019 · 19 comments
Open
21 of 59 tasks

MapKnitter Upgrades planning (overall) #300

jywarren opened this issue Jan 23, 2019 · 19 comments

Comments

@jywarren
Copy link
Member

jywarren commented Jan 23, 2019

MapKnitter upgrades being considered and planned fall into a few different categories:

  1. MapKnitter Exporter (MapKnitterExporter architecture discussion #298 and Collecting resources for MapKnitterExporter.js #296)
  2. Rails upgrades + testing
  3. MapKnitter UI

We'll break these out in more detail in separate issues, but do a brief overview here. For all planning issues, see:

https://github.com/publiclab/mapknitter/projects/1

Some of the changes will be aimed at infrastructure and sustainability, as we can't make big changes or improvements without, for example, better tests and a more up-to-date Rails stack. Others will be bug fixes and cleanup of the existing system. Finally, some changes will include a massive optimization of the export system backed by Google Cloud (or equivalent serverless backend), and some more serious UI changes to the map creation, login, and image upload workflows, as well as the map display pages.

A rough working diagram of MapKnitter 2.5 can be found here: https://docs.google.com/presentation/d/12NU6V9UFeOWKJrql8AIQZI3mKUWTuakF8zrGpOlgrNg/edit#slide=id.g49650b018b_0_0

Here are the overall projects; be aware that we may not prioritize each of these, and may try to focus on some of the most urgent and impactful for starters:

Stand-alone Cloud Exporter project

See #298 for in-depth discussion!

Rails upgrades

  • Tour the code and evaluate (@jywarren, others)
    • deprecate very old MapKnitter 1 templates, controllers (some done in Cleanup #313, more needed for old Knitter interface, etc)
    • compile list of routes/URLs to build functional tests around
    • vastly expand unit tests Planning issue: expand test coverage with more thorough testing #304
    • move lots into models for unit testing
    • get CI running (Travis)
    • add test coverage reporting Add dangerfile to the project #303
    • Map out JS API endpoints of MapKnitter, write functional tests for them:
      • image upload
      • image positioning (corner coordinates)
      • image display at diff. sizes
      • saveImage: function() {
        //console.log('saving')
        var img = this
        // reset change state string:
        img._corner_state = JSON.stringify(img._corners)
        // send save request
        $.ajax('/images/update',{
        type: 'POST',
        data: {
        warpable_id: img.warpable_id,
        locked: (img.editing._mode == 'lock'),
        points:
        img._corners[0].lng+','+img._corners[0].lat+':'+
        img._corners[1].lng+','+img._corners[1].lat+':'+
        img._corners[3].lng+','+img._corners[3].lat+':'+
        img._corners[2].lng+','+img._corners[2].lat,
        },
        beforeSend: function(e) {
        $('.mk-save').removeClass('fa-check-circle fa-times-circle fa-green fa-red').addClass('fa-spinner fa-spin')
        },
        complete: function(e) {
        $('.mk-save').removeClass('fa-spinner fa-spin').addClass('fa-check-circle fa-green')
        },
        error: function(e) {
        $('.mk-save').removeClass('fa-spinner fa-spin').addClass('fa-times-circle fa-red')
        }
        })
        },
        // /maps/newbie/warpables/42, but we'll try /warpables/42
        // as it should also be a valid restful route
        deleteImage: function() {
        var img = this
        // this should only be possible by logged-in users
        if (mapKnitter.logged_in) {
        if (confirm("Are you sure you want to delete this image? You cannot undo this.")) {
        $.ajax('/images/'+img.warpable_id,{
        dataType: "json",
        type: 'DELETE',
        beforeSend: function(e) {
        $('.mk-save').removeClass('fa-check-circle fa-times-circle fa-green fa-red').addClass('fa-spinner fa-spin')
        },
        complete: function(e) {
        $('.mk-save').removeClass('fa-spinner fa-spin').addClass('fa-check-circle fa-green')
        // disable interactivity:
        img.editing._hideToolbar();
        img.editing.disable();
        // remove from Leaflet map:
        map.removeLayer(img);
        // remove from sidebar too:
        $('#warpable-'+img.warpable_id).remove()
        },
        error: function(e) {
        $('.mk-save').removeClass('fa-spinner fa-spin').addClass('fa-times-circle fa-red')
        }
        })
        }
        } else {
        alert('You must be logged in to delete images.')
        }
        },
        ,
        /* Load warpables data via AJAX request. */
        this._warpablesUrl = options.warpablesUrl;
        this.withWarpables(function(warpables){
        $.each(warpables,function(i,warpable) {
        // only already-placed images:
        if (warpable.nodes.length > 0) {
        var downloadEl = $('.img-download-' + warpable.id),
        imgEl = $('#full-img-' + warpable.id);
        downloadEl.click(function() {
        downloadEl.html("<i class='fa fa-circle-o-notch fa-spin'></i>");
        imgEl[0].onload = function() {
        var height = imgEl.height(),
        width = imgEl.width(),
        nw = map.latLngToContainerPoint(warpable.nodes[0]),
        ne = map.latLngToContainerPoint(warpable.nodes[1]),
        se = map.latLngToContainerPoint(warpable.nodes[2]),
        sw = map.latLngToContainerPoint(warpable.nodes[3]),
        offsetX = nw.x,
        offsetY = nw.y,
        displayedWidth = $('#warpable-img-' + warpable.id).width(),
        ratio = width / displayedWidth;
        nw.x -= offsetX;
        ne.x -= offsetX;
        se.x -= offsetX;
        sw.x -= offsetX;
        nw.y -= offsetY;
        ne.y -= offsetY;
        se.y -= offsetY;
        sw.y -= offsetY;
        warpWebGl(
        'full-img-' + warpable.id,
        [ 0, 0, width, 0, width, height, 0, height ],
        [ nw.x, nw.y, ne.x, ne.y, se.x, se.y, sw.x, sw.y ],
        true // trigger download
        )
        downloadEl.html("<i class='fa fa-download'></i>");
        }
        imgEl[0].src = $('.img-download-' + warpable.id).attr('data-image');
        });
        var corners = [
        new L.latLng(warpable.nodes[0].lat,
        warpable.nodes[0].lon),
        new L.latLng(warpable.nodes[1].lat,
        warpable.nodes[1].lon),
        new L.latLng(warpable.nodes[3].lat,
        warpable.nodes[3].lon),
        new L.latLng(warpable.nodes[2].lat,
        warpable.nodes[2].lon)
        ];
        var img = new L.DistortableImageOverlay(
        warpable.srcmedium,
        {
        corners: corners,
        mode: 'lock'
        }).addTo(window.mapKnitter._map);
        bounds = bounds.concat(corners);
        window.mapKnitter._map.fitBounds(bounds);
        images.push(img);
        img.warpable_id = warpable.id
        if (!options.readOnly) {
        // img.on('select', function(e){
        // refactor to use on/fire; but it doesn't seem to work
        // without doing it like this:
        L.DomEvent.on(img._image, 'click', window.mapKnitter.selectImage, img);
        img.on('deselect', window.mapKnitter.saveImageIfChanged, img)
        L.DomEvent.on(img._image, 'dblclick', window.mapKnitter.dblClickImage, img);
        L.DomEvent.on(img._image, 'load', function() {
        var img = this
        window.mapKnitter.setupToolbar(img)
        }, img);
        }
        }
        });
        });
        /* Deselect images if you click on the sidebar,
        * otherwise hotkeys still fire as you type. */
        $('.sidebar').click(function(){ $.each(images,function(i,img){ img.editing.disable() }) })
        /* Deselect images if you click on the map. */
        //this._map.on('click',function(){ $.each(images,function(i,img){ img.editing.disable() }) })
        // hi res:
        //img._image.src = img._image.src.split('_medium').join('')
        },
    • refactor pagination code?
    • consolidate and reorganize editor vs. viewer vs. maps listing templates, exporter templates
    • expand test fixtures
    • attempt to remove all Way and Node code (maybe done in Cleanup #313)
  • Test coverage (Planning issue: expand test coverage with more thorough testing #304)
  • Rails 3 => 5.1 upgrade Planning Issue: Upgrade rails from 3.2 to rails 6 #305
  • MapKnitter/Plots2 integration: Discussion: integrating or merging Mapknitter into plots2/PublicLab.org #302

New Rails dev

Image position versioning, latency, etc

  • Rename Warpable to Image?
  • New model ImagePosition, or ImagePlacement? with corner coordinates, user_id, timestamp
  • (further along) plan/conceptualize image update rejection (based on locking or whatever) and a UI display of edit rejection
  • low-latency plan for real-time multi user image placement (locking/unlocking/transactions)
  • give images a parent column so we can track multiple versions of an image and also use image sequencer to modify them and re-upload...
  • sockets? may need to work upstream in https://github.com/publiclab/Leaflet.DistortableImage/... not sure
  • simultaneous multi-user UI, like Google docs
  • edit history/reversion, accept changes -- how to present UI? "list/history" menu?
  • hard to see which images have been recently - may be uploading over and over (pre-detect this?)

Leaflet.DistortableImage upgrades

Moving most to here: publiclab/Leaflet.DistortableImage#126

  • Leaflet.DistortableImage
    • multiple image selection
    • simpler menu API to add new tools (even after initialization)
    • debugging of menu placement, hide/show (list out bugs)
    • full res download from menu (clean copy of #97 full resolution download Leaflet.DistortableImage#100 but needs a way to pass in full-res original image)
    • 'place with geodata' tool, not sure if this lives in MK or LDI, we have some code for it, need to link to it from here
    • nested menus or modal for more tools?
  • ordering? by a layers tab?
    • ability to set order in Leaflet DistortableImage (use z-index) using function like map.order_by(f()); integrate with MK UI
  • toolbar is a mess depending on browser
  • toolbar issues when you delete things

User Interface (brainstorm here, needs organizing)

  • more clearly frame as "Edit the base map"? workflow to Google Maps?
  • great presentation of maps - a listing page featuring nearby/related images, maps, people, info (MapKnitter Front Page UI discussion #327)
    • Featuring related work and groups below, nearby people and prompt to reach out
  • A starting page with a menu: kite/balloon/pole/drone/window with drawings and a "start uploading images" button (also MapKnitter Front Page UI discussion #327)
  • Login flow improvements (OAuth integration?)
  • UI improvements for Editor
    • combine image display from sidebar vs. modal, simplify
    • reorderable image sidebar
  • UI self-testing to ID issues
  • not clear who can edit what; what happens if i open someone's map and edit it (have image locking defaults, and edit history/reversion)
  • image upload is slow (look at FileUpload UI)
  • could we begin placing images even before upload is complete? (would we need to save locally in the browser?)
  • interactive walkthrough? tour? showing by doing

Leaflet Environmental Layers

  • improved layers browser
  • "add your data"
  • MapKnitter images layer, or the MK layer displaying individual images at high enough zoom levels

Big ideas

These ideas will likely be too large for this project but let's think about them and plan for the future.

  • “Just start uploading images”
  • Starts with map of current location as homepage
  • Lives on PublicLab.org, or uses PL login (as now)?
  • Based around coords, not map names
@jywarren
Copy link
Member Author

This is pretty big, folks! Just a heads up to @icarito @tech4GT @SidharthBansal @gauravano that this is coming up pretty soon. Interested to hear your input!

@SidharthBansal
Copy link
Member

SidharthBansal commented Jan 25, 2019

I think we should write project titles in this issue and open up issues for each of the above to discuss. If we will discuss projects in this issue then things will become confusing, overlapping, conflicting and will result in diverge discussions.
Let's break the above into individual issues and discuss each project mentioned by Jeff above in its separate issue.
Few points which I observe:

  • Low number of collaborators at mapknitter as compared to plots2. So we need to shift the diversity from plots2 to this repo
  • Listing major ideas related to Mapknitter in this issue
  • Creating Milestones for each of them.
  • After 15 days GSoC & RGSoC proposals will start coming so we need to decide on the projects a little early so that new folks can start writing their proposals.
  • maintainability issues like documentation/outreachy in mapknitter repository. Enhancement of readme, wiki pages, milestone pages, labels etc at this repository(even though it seems to be a small task but for the entire repository it is a big task).

@SidharthBansal
Copy link
Member

Other ideas:

  • Addition of danger bot
  • Addition of codeclimate
  • Addition of dependabot

@grvsachdeva
Copy link
Member

grvsachdeva commented Jan 25, 2019

Hey @jywarren @SidharthBansal,

There are some projects which can be formed from this issue like Rails upgrade, User Interface, etc. I think we should divide this one into more planning issues.

Regarding installation of dangerbot, codeclimate and dependabot I have opened issue Sidharth.
Thanks!

@jywarren
Copy link
Member Author

jywarren commented Jan 25, 2019 via email

@SidharthBansal
Copy link
Member

I am trying to shift some portion of the crowd from plots2 to mapknitter also so that this repository will become active as plots2, image-sequencer is. I hope @jywarren, @gauravano @ebarry you will like this.

@jywarren
Copy link
Member Author

Great work.

@jywarren
Copy link
Member Author

jywarren commented Feb 1, 2019

Made a project to track some of this work!!!

@jywarren
Copy link
Member Author

jywarren commented Feb 4, 2019

Opened a more detailed list here for Leaflet.DistortableImage, the UI for map knitting: publiclab/Leaflet.DistortableImage#126

@jywarren
Copy link
Member Author

jywarren commented Feb 6, 2019

@kayrufty wrote in with some great UI notes -- just noting to Kay that we have issues open for some, and not others. I'll note connected issues where they exist, and we can break out new ones as we go. THANKS!!!

  • Lock background: On a couple occasions I accidentally moved the google maps background and it was difficult to get it back perfectly aligned with my pictures.

...actually this one, i'm a bit surprised that it can become unlinked with the images. any clues or screenshots on how this happened, or what led up to it so we can try to reproduce it? Thanks!

  • Blink images on or off: There is already a feature to make images more transparent, but it might be handy to have it go as far as to disappear except for maybe an outline so you can more clearly see the google maps background. (Refactor image border drawing (css border is affected by image transparency) Leaflet.DistortableImage#6)
  • Coordinates: It might be useful to have access to the coordinates from google maps to help identify specific locations that are known on images. (adding this to the above list of UI fixes)
  • Zoom Control: I was zooming in and out using my scroll wheel and there were times when I couldn't quite get the right magnification and would have liked a physical magnification bar or even an area to input a number %. (this too!)
  • I noticed when I exported the images to a .jpg after manipulating them the images were reordered from what I’d done in mapknitter and it brought forward an image I wasn’t able to match well to the landscape. Example of screenshot of the map knitter page and the shifted image file are attached. (Most obvious if you look at the road in the upper right) (ordering -- client-side image ordering interface #116)

@IshaGupta18
Copy link

Yes I think this would be very much needed to make MapKnitter as good as plots2. I am interested in tackling and brainstorming on some of these projects,especially the ones related to MapKnitter UI and I also wanted to chime in some suggestions for UI design for it to be of efficient and smooth use to the public, as I think a lot of brainstorming can be done on making the website easy to use and better organised. I will try to open some planning issues regarding the same for us to discuss there. Does this sound okay @jywarren ?

@jywarren
Copy link
Member Author

jywarren commented Feb 7, 2019

Hi @IshaGupta18 -- thanks, that would be great! If you'd like to chime in on the Exporter UI discussion here, that'd be awesome: #326

I think we could also start one called MapKnitter Editor UI discussion where we can discuss the basic editor layout here: https://mapknitter.org/maps/cranston-test (say, UI for the Uploading, Image listing, Exporting, and the main Leaflet map editing areas). We could copy in some of these issues for discussion there:

* [ ]  image collections/groups rather than "maps"  
  * [ ]  ability to see all images (faded maybe?) for a region and "add to this collection/map" by clicking
* [ ]  view of all images for given region in maps (faded/greyed if not part of this map)
* [ ]  Ability to choose time bounding box (some kind of layer manager maybe needed)
* [ ]  Ability to toggle viewing your images or all images (uploads by other users)
* [ ]  Ability to select multiple images for export " a collection"

@jywarren
Copy link
Member Author

jywarren commented Feb 7, 2019

Also note that I've built in a few of these projects with more detail on our Summer of Code program Ideas page: https://publiclab.org/gsoc-ideas, including:

  • MapKnitter Synchronous Editing
  • MapKnitter Cloud Exporter
  • MapKnitter UI (a collection of projects)
  • MapKnitter Image Management

@Divy123
Copy link
Member

Divy123 commented Feb 7, 2019

@jywarren one thing that I went through the ideas for image-sequencer project.
Is that a single idea that is mentioned there under gsoc-ideas or are they multiple?

@jywarren
Copy link
Member Author

jywarren commented Feb 7, 2019 via email

@Divy123
Copy link
Member

Divy123 commented Feb 7, 2019

Thanks .

@IshaGupta18
Copy link

@jywarren This would be lovely I think. I will go through all of this is deeper details. Thanks a lot!

@SidharthBansal
Copy link
Member

SidharthBansal commented Feb 10, 2019 via email

@jywarren
Copy link
Member Author

jywarren commented Apr 6, 2019

Added more info on Collections model which would be related many-to-many to Images, instead of Warpables that belong to only one Map.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants