Add asychronous loading to map views #948
Conversation
|
I'd like to confirm that this PR still loads everything regardless of the state of the map's location and zoom level, right? The difference is that before there is a blank space while the data is being processed, but now there is an initial map with a "Loading..." alert. |
|
Yes, that is correct. The main difference is that the page loads first and then the features are loaded onto the map in batches of 1,000. |
oliverroick
force-pushed
the
feature/#931-async-loading
branch
3 times, most recently
from
44dbb5b to
44655b8
Compare
|
@seav Any news on this? Does it need more work or is it good to go? |
|
Still reviewing the actual code. It is a pretty big update. But I will finish this today so that it can go into QA/code freeze. |
|
|
||
| var trans = {open: "{% trans 'Open location' %}"} | ||
| renderFeatures(detail.map, null, data, trans); | ||
| renderFeatures(detail.map, |
There was a problem hiding this comment.
Before, trans was created but it was now removed. But in other templates, trans was still retained. Which should it be?
There was a problem hiding this comment.
When you edit a feature, you don't want to open other locations from the map. Popups are not provided on the edit, hence the translation is not needed. This condition checks whether to display to popup or not.
| else: | ||
| self._queryset = self.proj.spatial_units.all().only( | ||
| 'id', 'type', 'geometry', 'project') | ||
| self._queryset = self.proj.spatial_units.all().only( |
There was a problem hiding this comment.
Since self.queryset_excludes_object was removed here, then the reference to it at SpatialUnitObjectMixin should also be removed.
| geoJson.addData(response); | ||
|
|
||
| if (response.next) { | ||
| loadFeatures(response.next, map, options.trans); |
There was a problem hiding this comment.
Would it be better to have some interval between loading pages?
There was a problem hiding this comment.
Why? The next request never starts before the previous one is finished.
There was a problem hiding this comment.
I was thinking that maybe there should be a pause between processing so the JS engine is not executing all the time. But I saw that there is a pause anyway because there is a network call. So this should be okay.
- Paginated spatial unit list views - Remove spatial units from template context - Add async loading to templates - Add loading indicator
oliverroick
force-pushed
the
feature/#931-async-loading
branch
from
44655b8 to
bcf2881
Compare
Proposed changes in this pull request
spatial.views.asyc.SpatialUnitList, which provides paginated access to spatial units on a project. The page size is set to 1000. The view uses a customPaginatorbased onGeoJsonPaginationorganization.views.default.ProjectDashboardspatial.views.default.LocationListspatial.views.default.LocationAddspatial.views.default.LocationDetailspatial.views.default.LocationEditparty.views.default.PartyRelationshipDetailparty.views.default.PartyRelationshipEditparty.views.default.PartyRelationshipDeleteparty.views.default.PartyRelationshipResourceAddparty.views.default.PartyRelationshipResourceNewspatial.views.asyc.SpatialUnitList.renderfeatures()insidecore/static/js/map_utils.jshas been adapted accordingly.templates/core/base.htmland hidden by default.renderfeatures()shows and hides the alert.When should this PR be merged
Anytime for sprint 11.
Risks
Low.
Follow up actions
While this PR should improve the responsiveness of map views, it will likely be only a stop-gap solution until we have implemented a more sustainable solution. Research into a sustainable solution should start now, and we should coordinate this with work towards improving map interactions.