From 087867d5c8e7eb5d0da3f088b4fb45d07b91c78e Mon Sep 17 00:00:00 2001 From: SilentSpike Date: Tue, 4 Feb 2020 21:05:40 +0000 Subject: [PATCH] Use Promises for Osmose issue detail requests Also ES6ify the details script --- modules/services/osmose.js | 30 ++-- modules/ui/osmose_details.js | 336 ++++++++++++++++++----------------- 2 files changed, 185 insertions(+), 181 deletions(-) diff --git a/modules/services/osmose.js b/modules/services/osmose.js index 4997ddb2f..e34260e0e 100644 --- a/modules/services/osmose.js +++ b/modules/services/osmose.js @@ -160,30 +160,26 @@ export default { }); }, - loadErrorDetail(d, callback) { + loadErrorDetail(d) { // Error details only need to be fetched once if (d.elems !== undefined) { - if (callback) callback(null, d); - return; + return Promise.resolve(d); } - let url = `${_osmoseUrlRoot}/issue/${d.identifier}?langs=${currentLocale}`; + const url = `${_osmoseUrlRoot}/issue/${d.identifier}?langs=${currentLocale}`; + const cacheDetails = data => { + // Associated elements used for highlighting + // Assign directly for immediate use in the callback + d.elems = data.elems.map(e => e.type.substring(0,1) + e.id); - d3_json(url) - .then(data => { - // Associated elements used for highlighting - // Assign directly for immediate use in the callback - d.elems = data.elems.map(e => e.type.substring(0,1) + e.id); + // Some issues have instance specific detail in a subtitle + d.detail = data.subtitle; - // Some issues have instance specific detail in a subtitle - d.detail = data.subtitle; + this.replaceError(d); + }; - this.replaceError(d); - if (callback) callback(null, d); - }) - .catch(err => { - if (callback) callback(err.message); - }); + return jsonPromise(url, cacheDetails) + .then(() => d); }, loadStrings(callback, locale=currentLocale) { diff --git a/modules/ui/osmose_details.js b/modules/ui/osmose_details.js index 7183810e5..a27e08cb1 100644 --- a/modules/ui/osmose_details.js +++ b/modules/ui/osmose_details.js @@ -1,6 +1,6 @@ import { - event as d3_event, - select as d3_select + event as d3_event, + select as d3_select } from 'd3-selection'; import { modeSelect } from '../modes/select'; @@ -10,190 +10,198 @@ import { utilDisplayName, utilEntityOrMemberSelector } from '../util'; export function uiOsmoseDetails(context) { - let _error; - const unknown = t('inspector.unknown'); + let _error; + const unknown = t('inspector.unknown'); - function issueString(d, type) { - if (!d) return unknown; + function issueString(d, type) { + if (!d) return unknown; - // Issue description supplied by Osmose - var s = services.osmose.getStrings(d.error_type); - return (type in s) ? s[type] : unknown; + // Issue description supplied by Osmose + const s = services.osmose.getStrings(d.error_type); + return (type in s) ? s[type] : unknown; + } + + + function osmoseDetails(selection) { + const details = selection.selectAll('.error-details') + .data( + _error ? [_error] : [], + d => `${d.id}-${d.status || 0}` + ); + + details.exit() + .remove(); + + const detailsEnter = details.enter() + .append('div') + .attr('class', 'error-details error-details-container'); + + + // Description + const descriptionDiv = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); + + descriptionDiv + .append('h4') + .text(() => t('QA.keepRight.detail_description')); + + descriptionDiv + .append('p') + .attr('class', 'error-details-description-text') + .html(d => issueString(d, 'detail')); + + // Elements (populated later as data is requested) + const detailsDiv = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); + + const elemsDiv = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); + + // Suggested Fix (musn't exist for every issue type) + if (issueString(_error, 'fix') !== unknown) { + let div = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); + + div + .append('h4') + .text(() => t('QA.osmose.fix_title')); + + div + .append('p') + .html(d => issueString(d, 'fix')); } + // Common Pitfalls (musn't exist for every issue type) + if (issueString(_error, 'trap') !== unknown) { + let div = detailsEnter + .append('div') + .attr('class', 'error-details-subsection'); - function osmoseDetails(selection) { - var details = selection.selectAll('.error-details') - .data( - _error ? [_error] : [], - d => `${d.id}-${d.status || 0}` - ); + div + .append('h4') + .text(() => t('QA.osmose.trap_title')); - details.exit() - .remove(); + div + .append('p') + .html(d => issueString(d, 'trap')); + } - var detailsEnter = details.enter() - .append('div') - .attr('class', 'error-details error-details-container'); + detailsEnter + .append('div') + .attr('class', 'translation-link') + .append('a') + .attr('target', '_blank') + .attr('rel', 'noopener noreferrer') // security measure + .attr('href', 'https://www.transifex.com/openstreetmap-france/osmose') + .text(() => t('QA.osmose.translation')) + .append('svg') + .attr('class', 'icon inline') + .append('use') + .attr('href', '#iD-icon-out-link'); + services.osmose.loadErrorDetail(_error) + .then(d => { + // No details to add if there are no associated issue elements + if (!d.elems || d.elems.length === 0) return; - var descriptionEnter = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); + // TODO: Do nothing if UI has moved on by the time this resolves - // Description - descriptionEnter + // Things like keys and values are dynamically added to a subtitle string + if (d.detail) { + detailsDiv .append('h4') - .text(() => t('QA.keepRight.detail_description')); + .attr('class', 'error-details-subtitle') + .text(() => t('QA.osmose.detail_title')); - descriptionEnter + detailsDiv .append('p') - .attr('class', 'error-details-description-text') - .html(d => issueString(d, 'detail')); - - // Elements (populated later as data is requested) - const detailsDiv = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); - - const elemsDiv = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); - - // Suggested Fix (musn't exist for every issue type) - if (issueString(_error, 'fix') !== unknown) { - let div = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); - - div.append('h4') - .text(() => t('QA.osmose.fix_title')); - - div.append('p') - .html(d => issueString(d, 'fix')); + .html(d => d.detail); } - // Common Pitfalls (musn't exist for every issue type) - if (issueString(_error, 'trap') !== unknown) { - let div = detailsEnter - .append('div') - .attr('class', 'error-details-subsection'); + // Create list of linked issue elements + elemsDiv + .append('h4') + .attr('class', 'error-details-subtitle') + .text(() => t('QA.osmose.elems_title')); - div.append('h4') - .text(() => t('QA.osmose.trap_title')); + elemsDiv + .append('ul') + .attr('class', 'error-details-elements') + .selectAll('.error_entity_link') + .data(d.elems) + .enter() + .append('li') + .append('a') + .attr('class', 'error_entity_link') + .text(d => d) + .each(function() { + const link = d3_select(this); + const entityID = this.textContent; + const entity = context.hasEntity(entityID); - div.append('p') - .html(d => issueString(d, 'trap')); - } + // Add click handler + link + .on('mouseenter', () => { + context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) + .classed('hover', true); + }) + .on('mouseleave', () => { + context.surface().selectAll('.hover') + .classed('hover', false); + }) + .on('click', () => { + d3_event.preventDefault(); + const osmlayer = context.layers().layer('osm'); + if (!osmlayer.enabled()) { + osmlayer.enabled(true); + } - detailsEnter - .append('div') - .attr('class', 'translation-link') - .append('a') - .attr('target', '_blank') - .attr('rel', 'noopener noreferrer') // security measure - .attr('href', 'https://www.transifex.com/openstreetmap-france/osmose') - .text(d => t('QA.osmose.translation')) - .append('svg') - .attr('class', 'icon inline') - .append('use') - .attr('href', '#iD-icon-out-link'); + context.map().centerZoom(d.loc, 20); - services.osmose.loadErrorDetail(_error, (err, d) => { - // No details to add if there are no associated issue elements - if (!d.elems || d.elems.length === 0) return; - - // Things like keys and values are dynamically added to a subtitle string - if (d.detail) { - detailsDiv - .append('h4') - .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.detail_title')); - - detailsDiv - .append('p') - .html(d => d.detail); - } - - // Create list of linked issue elements - elemsDiv - .append('h4') - .attr('class', 'error-details-subtitle') - .text(() => t('QA.osmose.elems_title')); - - elemsDiv - .append('ul') - .attr('class', 'error-details-elements') - .selectAll('.error_entity_link') - .data(d.elems) - .enter() - .append('li') - .append('a') - .attr('class', 'error_entity_link') - .text(d => d) - .each(function() { - var link = d3_select(this); - var entityID = this.textContent; - var entity = context.hasEntity(entityID); - - // Add click handler - link - .on('mouseenter', () => { - context.surface().selectAll(utilEntityOrMemberSelector([entityID], context.graph())) - .classed('hover', true); - }) - .on('mouseleave', () => { - context.surface().selectAll('.hover') - .classed('hover', false); - }) - .on('click', function() { - d3_event.preventDefault(); - var osmlayer = context.layers().layer('osm'); - if (!osmlayer.enabled()) { - osmlayer.enabled(true); - } - - context.map().centerZoom(d.loc, 20); - - if (entity) { - context.enter(modeSelect(context, [entityID])); - } else { - context.loadEntity(entityID, function() { - context.enter(modeSelect(context, [entityID])); - }); - } - }); - - // Replace with friendly name if possible - // (The entity may not yet be loaded into the graph) - if (entity) { - var name = utilDisplayName(entity); // try to use common name - - if (!name) { - var preset = context.presets().match(entity, context.graph()); - name = preset && !preset.isFallback() && preset.name(); // fallback to preset name - } - - if (name) { - this.innerText = name; - } - } + if (entity) { + context.enter(modeSelect(context, [entityID])); + } else { + context.loadEntity(entityID, () => { + context.enter(modeSelect(context, [entityID])); + }); + } }); - // Don't hide entities related to this error - #5880 - context.features().forceVisible(d.elems); - context.map().pan([0,0]); // trigger a redraw - }); - } - - - osmoseDetails.error = (val) => { - if (!arguments.length) return _error; - _error = val; - return osmoseDetails; - }; + // Replace with friendly name if possible + // (The entity may not yet be loaded into the graph) + if (entity) { + let name = utilDisplayName(entity); // try to use common name + + if (!name) { + const preset = context.presets().match(entity, context.graph()); + name = preset && !preset.isFallback() && preset.name(); // fallback to preset name + } + + if (name) { + this.innerText = name; + } + } + }); + + // Don't hide entities related to this error - #5880 + context.features().forceVisible(d.elems); + context.map().pan([0,0]); // trigger a redraw + }) + .catch(err => {}); // TODO: Handle failed json request gracefully in some way + } + osmoseDetails.error = val => { + if (!arguments.length) return _error; + _error = val; return osmoseDetails; -} \ No newline at end of file + }; + + + return osmoseDetails; +}