Ht 38 select task #51
Conversation
| } | ||
|
|
||
| vm.selectRandomTask = function () { |
| var layers = vm.map.getLayers(); | ||
| for (var i = 0; i < layers.getLength(); i++) { | ||
| if (layers.item(i).get('name') === 'tasks') { | ||
| var feature = layers.item(i).getSource().getFeatures().filter(function (feature) { |
There was a problem hiding this comment.
Instead of iterating over the layers, you could store the vector layer as a controller variable and reference it directly.
| select.getFeatures().clear(); | ||
| select.getFeatures().push(feature); | ||
| onTaskSelection(feature); | ||
| var vPadding = vm.map.getSize()[1] * 0.3; |
There was a problem hiding this comment.
Explain with a comment what this magic number is exactly
…nto ht-38-select-task
|
@LindaAlblas OK, looking to merge now, if you could have a good look. Thanks. |
|
@LindaAlblas for the comment on the merge can we please discuss when you get to that stage. Thanks. |
|
@feenster , working on it! |
| vm.mappingStep = 'view'; | ||
| }, function () { | ||
| // task not returned successfully | ||
| // TODO - may want to handle error |
There was a problem hiding this comment.
The error is handled so we can remove the TODO
| @@ -86,5 +144,31 @@ | |||
| }); | |||
| source.addFeature(aoiFeatures); | |||
| } | |||
|
|
|||
| /** | |||
| * Gets a task from the server and uses sets up the task returned as the currently selected task | |||
client/app/project/project.html
Outdated
| <h3>*Mapping*</h3> | ||
| <button class="button button--secondary">Start Mapping</button> | ||
| <div class="content"> | ||
| <div class="button-group button-group--horizontal" role="group" aria-label="..."> |
There was a problem hiding this comment.
it is good practice to put a description in the aria-label
|
|
||
| // read tasks JSON into features | ||
| console.log(tasks); |
| */ | ||
| function initialiseProject(id){ | ||
| vm.selectRandomTask = function () { | ||
| var feature = taskService.getRandomMappableTaskFeature(vm.taskVectorLayer.getSource().getFeatures()); |
There was a problem hiding this comment.
Looks a lot neater now 👍
client/app/project/project.html
Outdated
| ng-hide="projectCtrl.selectedTask.taskLocked"> {{ projectCtrl.selectedTask.taskStatus }} </span> | ||
| </h3> | ||
|
|
||
|
|
client/app/services/task.service.js
Outdated
| */ | ||
| function getRandomMappableTaskFeature(features) { | ||
| //first check that we have a non empty array of ol.Feature objects | ||
| if (features && (Object.prototype.toString.call(features) === '[object Array]') && features.length > 0) { |
There was a problem hiding this comment.
could you use features instanceof Array instead of Object.prototype? Or would if (features && features.length > 0) even be enough?
client/app/services/task.service.js
Outdated
| */ | ||
| function getTasksByStatus(features, taskLocked, taskStatus) { | ||
| candidates = []; | ||
| if (features && (Object.prototype.toString.call(features) === '[object Array]') && features.length > 0) { |
client/app/services/task.service.js
Outdated
| // safe to use the function | ||
| var isLocked = item.get('taskLocked'); | ||
| var status = item.get('taskStatus'); | ||
| if (item.get('taskLocked') == taskLocked && item.get('taskStatus') === taskStatus) return item; |
There was a problem hiding this comment.
You have declared the variables isLocked and status so you can use them. Makes it easier to read too :)
if (isLocked == taskLocked && status == taskStatus){
return item;
}
| @@ -0,0 +1,254 @@ | |||
| 'use strict'; | |||
|
|
|||
| describe('task.service', function () { | |||
|
@LindaAlblas that's the requested changes done |
fix: UI fixes
Looking for a first review of the code please