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.

Sign up for GitHub

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

Jump to bottom

Fix issue #84: Change feature count tracking to operate across AI source types #85

Merged
merged 4 commits into from
Dec 10, 2019
Merged

Fix issue #84: Change feature count tracking to operate across AI source types #85

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
095505a
Fix AI Feature maximum-count-per-changeset logic to include all AI fe…
Bonkles Dec 9, 2019
0558b29
Fix lint errors.
Bonkles Dec 9, 2019
6c4e1c4
Address PR feedback by reducing code changes to core history module.
Bonkles Dec 9, 2019
3c2fad4
One more round of PR feedback! use array method to tighten up code, s…
Bonkles Dec 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions data/core.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,8 @@ en:
description: Does this look like an accurate feature? Select this to start editing it so that you can connect, tag, and save it to OpenStreetMap. 👍
annotation: Added a Map With AI feature.
tooltip: Add this feature to the map to begin editing.
disabled: To help improve OSM data quality, we only allow {n} ML roads to be added in each mapping session. Please try to fix issues on your edits so far. You will be able to add more roads after saving.
disabled_flash: To help improve OSM data quality, we only allow {n} ML roads to be added in each mapping session.
disabled: To help improve OSM data quality, we only allow {n} AI features to be added in each mapping session. Please try to fix issues on your edits so far. You will be able to add more AI features after saving.
disabled_flash: To help improve OSM data quality, we only allow {n} AI Features to be added in each mapping session.
key: A
option_reject:
label: Remove This Feature
Expand Down
29 changes: 15 additions & 14 deletions modules/ui/fb_feature_picker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,37 @@ import { uiRapidFirstEdit } from './rapid_first_edit_dialog';

export function uiFbFeaturePicker(context, keybinding) {
var _datum;
var ML_ROADS_LIMIT_NON_TM_MODE = 50;
var AI_FEATURES_LIMIT_NON_TM_MODE = 50;


function isAddRoadDisabled() {
function isAddFeatureDisabled() {
// when task GPX is set in URL (TM mode), "add roads" is always enabled
var gpxInUrl = utilStringQs(window.location.hash).gpx;
if (gpxInUrl) return false;

var mlRoadsCount = 0;
var entities = context.graph().entities;
for (var eid in entities) {
var e = entities[eid];
if (eid.startsWith('w-') && e && (e.tags.source === 'digitalglobe' || e.tags.source === 'maxar')) {
mlRoadsCount += 1;
var aiFeatures = context.history().peekAllAnnotations();
Bonkles marked this conversation as resolved.
Show resolved Hide resolved

var aiFeatureCount = 0;
for (var i = 0; i <= aiFeatures.length; i++) {
if (aiFeatures[i] && aiFeatures[i].type === 'fb_accept_feature') {
aiFeatureCount++;
}
}
return mlRoadsCount >= ML_ROADS_LIMIT_NON_TM_MODE;

return aiFeatureCount >= AI_FEATURES_LIMIT_NON_TM_MODE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annotations are guaranteed to exist from the core/history.js method, so you can safely drop the check for aiFeatures[i] in if (aiFeatures[i] && ...).

Also, it's effectively a filter & count, so if you want to get crazy:

var aiFeatureAccepts = annotations.filter(function (a) { return a.type === 'fb_accept_feature'; });
return aiFeatureAccepts.length >= AI_FEATURES_LIMIT_NON_TM_MODE;

Copy link
Contributor Author

@Bonkles Bonkles Dec 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

less code is better code! I keep having to remind myself that all the Javascript array methods (filter, map, reduce, etc) are supported in iD. For some reason I keep eschewing them, even though it's newer stuff like ES2016 'let', 'const', etc. that isn't supported.

}


function onAcceptRoad() {
if (_datum) {
if (isAddRoadDisabled()) {
if (isAddFeatureDisabled()) {
var flash = uiFlash()
.duration(4000)
.iconName('#iD-icon-rapid-plus-circle')
.iconClass('operation disabled')
.text(t(
'fb_feature_picker.option_accept.disabled_flash',
{n: ML_ROADS_LIMIT_NON_TM_MODE}
{n: AI_FEATURES_LIMIT_NON_TM_MODE}
));
flash();
return;
Expand Down Expand Up @@ -204,18 +205,18 @@ export function uiFbFeaturePicker(context, keybinding) {
.placement('bottom')
.html(true)
.title(function() {
return isAddRoadDisabled()
return isAddFeatureDisabled()
? uiTooltipHtml(t(
'fb_feature_picker.option_accept.disabled',
{n: ML_ROADS_LIMIT_NON_TM_MODE}
{n: AI_FEATURES_LIMIT_NON_TM_MODE}
))
: uiTooltipHtml(
t('fb_feature_picker.option_accept.tooltip'),
t('fb_feature_picker.option_accept.key')
);
}),
onClick: onAcceptRoad,
disabledFunction: isAddRoadDisabled
disabledFunction: isAddFeatureDisabled
}, 'ai-features-accept');

presetItem(bodyEnter, {
Expand Down
2 changes: 1 addition & 1 deletion modules/ui/rapid_feature_toggle_dialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export function uiRapidFeatureToggleDialog(context, AIFeatureToggleKey, featureT
toggleOptionText
.append('div')
.attr('class', 'rapid-feature-license')
.html(options.license)
.html(options.license);

toggleOptionText.select('p a')
.attr('target','_blank');
Expand Down