From 3a3d977f7e5f864c190d88986b311ca243a2f332 Mon Sep 17 00:00:00 2001 From: Kyle Hensel Date: Thu, 30 Sep 2021 22:17:27 +1300 Subject: [PATCH 1/2] add preview for colour input --- css/80_app.css | 12 +++++++++++- modules/ui/fields/address.js | 2 +- modules/ui/fields/input.js | 33 ++++++++++++++++++++++++++++++++- modules/util/get_set_value.js | 1 + 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 452fd224a..83a336655 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -1505,6 +1505,15 @@ a.hide-toggle { fill: #333; opacity: .5; } +.form-field-button.colour-preview { + background-color: #fff; + border-radius: 0 0 4px 0; +} +.form-field-button.colour-preview > div { + border: 3px solid #fff; + height: 100%; + border-radius: 8px; +} /* round corners of first/last child elements */ @@ -1703,12 +1712,13 @@ a.hide-toggle { /* Field - Text / Numeric ------------------------------------------------------- */ -.form-field-input-text > input:only-of-type, +.form-field-input-text > input:only-child, .form-field-input-tel > input:only-of-type, .form-field-input-email > input:only-of-type, .form-field-input-url > input:only-child { border-radius: 0 0 4px 4px; } +.form-field-input-text > input:not(:only-child), .form-field-input-url > input:not(:only-child) { border-radius: 0 0 0 4px; } diff --git a/modules/ui/fields/address.js b/modules/ui/fields/address.js index 783007929..377fb8ab3 100644 --- a/modules/ui/fields/address.js +++ b/modules/ui/fields/address.js @@ -288,7 +288,7 @@ export function uiFieldAddress(field, context) { }) .attr('title', function(subfield) { var val = tags[field.key + ':' + subfield.id]; - return val && Array.isArray(val) && val.filter(Boolean).join('\n'); + return (val && Array.isArray(val)) ? val.filter(Boolean).join('\n') : undefined; }) .classed('mixed', function(subfield) { return Array.isArray(tags[field.key + ':' + subfield.id]); diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index d5bec7405..099be53ae 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -21,6 +21,7 @@ export function uiFieldText(field, context) { var dispatch = d3_dispatch('change'); var input = d3_select(null); var outlinkButton = d3_select(null); + var wrap = d3_select(null); var _entityIDs = []; var _tags; var _phoneFormats = {}; @@ -62,7 +63,7 @@ export function uiFieldText(field, context) { calcLocked(); var isLocked = field.locked(); - var wrap = selection.selectAll('.form-field-input-wrap') + wrap = selection.selectAll('.form-field-input-wrap') .data([0]); wrap = wrap.enter() @@ -167,9 +168,37 @@ export function uiFieldText(field, context) { if (value) window.open(value, '_blank'); }) .merge(outlinkButton); + } else if (field.key.includes('colour')) { + input.attr('type', 'text'); + + updateColourPreview(); } } + function updateColourPreview() { + wrap.selectAll('.foreign-id-permalink') + .remove(); + + const colour = utilGetSetValue(input); + + // see https://github.com/openstreetmap/openstreetmap-website/blob/08e2a0/app/helpers/browse_tags_helper.rb#L173 + // we use the same logic to validate colours, except we don't need to check whether named colours + // are valid, since the browser does this natively when we set the background-colour + const isColourValid = !!colour.match(/^(#([0-9a-fA-F]{3}){1,2}|\w+)$/); + if (!isColourValid) return; + + outlinkButton = wrap.selectAll('.foreign-id-permalink') + .data([colour], d => d); + + outlinkButton + .enter() + .append('div') + .attr('class', 'form-field-button foreign-id-permalink colour-preview') + .append('div') + .style('background-color', d => d) + .merge(outlinkButton); + } + function updatePhonePlaceholder() { if (input.empty() || !Object.keys(_phoneFormats).length) return; @@ -247,6 +276,8 @@ export function uiFieldText(field, context) { .attr('placeholder', isMixed ? t('inspector.multiple_values') : (field.placeholder() || t('inspector.unknown'))) .classed('mixed', isMixed); + if (field.key.includes('colour')) updateColourPreview(); + if (outlinkButton && !outlinkButton.empty()) { var disabled = !validIdentifierValueForLink(); outlinkButton.classed('disabled', disabled); diff --git a/modules/util/get_set_value.js b/modules/util/get_set_value.js index d55f805ae..486a3902e 100644 --- a/modules/util/get_set_value.js +++ b/modules/util/get_set_value.js @@ -1,5 +1,6 @@ // Like selection.property('value', ...), but avoids no-op value sets, // which can result in layout/repaint thrashing in some situations. +/** @returns {string} */ export function utilGetSetValue(selection, value) { function d3_selection_value(value) { function valueNull() { From e4008b422985fa3b6334d3afb496ff1827684ac5 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Wed, 1 Dec 2021 11:14:32 +0100 Subject: [PATCH 2/2] show native colour picker when clicking on colour preview further tweaks: * hide the box on invalid colours * only affect field with a tag key that included "colour" separated by ":" --- css/80_app.css | 26 ++++++++++++++++--- modules/ui/fields/input.js | 53 +++++++++++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 83a336655..cb3449c41 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -189,7 +189,8 @@ input[type=number], input[type=url], input[type=tel], input[type=email], -input[type=date] { +input[type=date], +input[type=color] { /* need this since line-height interpretation may vary by font or browser */ height: 2.585em; } @@ -1506,13 +1507,32 @@ a.hide-toggle { opacity: .5; } .form-field-button.colour-preview { - background-color: #fff; border-radius: 0 0 4px 0; } -.form-field-button.colour-preview > div { +.form-field-button.colour-preview > div.colour-box { border: 3px solid #fff; height: 100%; border-radius: 8px; + cursor: pointer; + text-align: center; + line-height: 20px; + padding: 1px 0 0 1px; +} +.inspector-hover .form-field-button.colour-preview > div.colour-box { + border-color: #ececec; +} +.form-field-button.colour-preview:active > div.colour-box, +.form-field-button.colour-preview:focus > div.colour-box { + border-color: #f1f1f1; +} +@media (hover: hover) { + .form-field-button.colour-preview:hover > div.colour-box { + border-color: #f1f1f1; + } +} +.form-field-button.colour-selector { + visibility: hidden; + position: absolute; } diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index 099be53ae..7ce6513f9 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -1,5 +1,6 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { select as d3_select } from 'd3-selection'; +import _debounce from 'lodash-es/debounce'; import * as countryCoder from '@ideditor/country-coder'; import { presetManager } from '../../presets'; @@ -168,34 +169,62 @@ export function uiFieldText(field, context) { if (value) window.open(value, '_blank'); }) .merge(outlinkButton); - } else if (field.key.includes('colour')) { + } else if (field.key.split(':').includes('colour')) { input.attr('type', 'text'); updateColourPreview(); } } + function isColourValid(colour) { + if (!colour.match(/^(#([0-9a-fA-F]{3}){1,2}|\w+)$/)) { + // OSM only supports hex or named colors + return false; + } else if (!CSS.supports('color', colour) || ['unset', 'inherit', 'initial', 'revert'].includes(colour)) { + // see https://stackoverflow.com/a/68217760/1627467 + return false; + } + return true; + } function updateColourPreview() { - wrap.selectAll('.foreign-id-permalink') + wrap.selectAll('.colour-preview') .remove(); const colour = utilGetSetValue(input); - // see https://github.com/openstreetmap/openstreetmap-website/blob/08e2a0/app/helpers/browse_tags_helper.rb#L173 - // we use the same logic to validate colours, except we don't need to check whether named colours - // are valid, since the browser does this natively when we set the background-colour - const isColourValid = !!colour.match(/^(#([0-9a-fA-F]{3}){1,2}|\w+)$/); - if (!isColourValid) return; + if (!isColourValid(colour) && colour !== '') return; - outlinkButton = wrap.selectAll('.foreign-id-permalink') - .data([colour], d => d); + var colourSelector = wrap.selectAll('.colour-selector') + .data([0]); + outlinkButton = wrap.selectAll('.colour-preview') + .data([colour]); - outlinkButton + colourSelector + .enter() + .append('input') + .attr('type', 'color') + .attr('class', 'form-field-button colour-selector') + .attr('value', colour) + .on('input', _debounce(function(d3_event) { + d3_event.preventDefault(); + var colour = this.value; + if (!isColourValid(colour)) return; + utilGetSetValue(input, this.value); + change()(); + updateColourPreview(); + }, 100)); + + outlinkButton = outlinkButton .enter() .append('div') - .attr('class', 'form-field-button foreign-id-permalink colour-preview') + .attr('class', 'form-field-button colour-preview') .append('div') .style('background-color', d => d) + .attr('class', 'colour-box'); + if (colour === '') outlinkButton = outlinkButton + .call(svgIcon('#iD-icon-edit')); + outlinkButton + .on('click', () => wrap.select('.colour-selector').node().click()) .merge(outlinkButton); } @@ -276,7 +305,7 @@ export function uiFieldText(field, context) { .attr('placeholder', isMixed ? t('inspector.multiple_values') : (field.placeholder() || t('inspector.unknown'))) .classed('mixed', isMixed); - if (field.key.includes('colour')) updateColourPreview(); + if (field.key.split(':').includes('colour')) updateColourPreview(); if (outlinkButton && !outlinkButton.empty()) { var disabled = !validIdentifierValueForLink();