Skip to content

show (route) relation colours in lists and comboboxes #9424

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

Merged
tyrasd merged 3 commits into from
Nov 17, 2023
Merged

show (route) relation colours in lists and comboboxes #9424

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e3e942
show (route) relation `colour`s in lists and comboboxes
tyrasd Dec 12, 2022
beea585
Merge branch 'develop' into relation-colours
tyrasd Nov 14, 2023
20b72a2
only render route colours if the value is a valid color
tyrasd Nov 17, 2023
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
#### :tada: New Features
* Show a _remaining input length_ indicator and a warning if the maximum for OSM tags (typically, 255 characters) is exceeded ([#9390], [#9392] thanks [@alanb43], [#7943], [#9374])
#### :sparkles: Usability & Accessibility
* Show the color of (route) relations in the form of small colored circles in relation membership section and feature search results ([#9424])
#### :white_check_mark: Validation
#### :bug: Bugfixes
* Fix bug which made it impossible to change an object's preset from a sub-preset to the respective parents preset (e.g. from Driveway to Service Road) ([#9372])
Expand All @@ -65,6 +66,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
[#9397]: https://github.com/openstreetmap/iD/issues/9397
[#9413]: https://github.com/openstreetmap/iD/pull/9413
[#9423]: https://github.com/openstreetmap/iD/pull/9423
[#9424]: https://github.com/openstreetmap/iD/pull/9424
[@alanb43]: https://github.com/alanb43
[@Rewinteer]: https://github.com/Rewinteer

Expand Down
29 changes: 28 additions & 1 deletion css/80_app.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,7 @@ a.hide-toggle {
.ideditor[dir='rtl'] .field-label .label-text {
padding: 5px 10px 4px 0;
}
.field-label .label-text span {
.field-label .label-text {
white-space: nowrap;
}

Expand Down Expand Up @@ -2748,12 +2748,39 @@ img.tag-reference-wiki-image {
font-weight: normal;
padding-left: 10px;
}
.section-raw-member-editor .member-row .member-entity-name.has-colour::before,
.section-raw-membership-editor .member-row .member-entity-name.has-colour::before,
.feature-list .entity-name.has-colour::before,
.combobox-parent-relation .has-colour::before {
display: inline-block;
content: '';
margin-left: -5px;
margin-right: 5px;
border-style: solid;
border-width: 4px;
border-radius: 4px;
border-color: inherit;
}
.combobox-parent-relation .has-colour::before {
margin-left: 2px;
}

.ideditor[dir='rtl'] .section-raw-member-editor .member-row .member-entity-name,
.ideditor[dir='rtl'] .section-raw-membership-editor .member-row .member-entity-name {
padding-left:0;
padding-right: 10px;
}
.ideditor[dir='rtl'] .section-raw-member-editor .member-row .member-entity-name.has-colour::before,
.ideditor[dir='rtl'] .section-raw-membership-editor .member-row .member-entity-name.has-colour::before,
.ideditor[dir='rtl'] .feature-list .entity-name.has-colour::before {
margin-left: 5px;
margin-right: -5px;
}
.ideditor[dir='rtl'] .combobox-parent-relation .has-colour::before {
margin-left: 5px;
margin-right: 2px;
}


.form-field-input-member > input.member-role {
border-radius: 0 0 4px 4px;
Expand Down
2 changes: 2 additions & 0 deletions modules/ui/feature_list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ export function uiFeatureList(context) {
label
.append('span')
.attr('class', 'entity-name')
.classed('has-colour', d => d.entity && d.entity.type === 'relation' && d.entity.tags.colour)
.style('border-color', d => d.entity && d.entity.type === 'relation' && d.entity.tags.colour)
.text(function(d) { return d.name; });

enter
Expand Down
2 changes: 2 additions & 0 deletions modules/ui/sections/raw_member_editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ export function uiSectionRawMemberEditor(context) {
labelLink
.append('span')
.attr('class', 'member-entity-name')
.classed('has-colour', d => d.member.type === 'relation' && d.member.tags.colour)
.style('border-color', d => d.member.type === 'relation' && d.member.tags.colour)
.text(function(d) { return utilDisplayName(d.member); });

label
Expand Down
32 changes: 28 additions & 4 deletions modules/ui/sections/raw_membership_editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,31 +239,53 @@ export function uiSectionRawMembershipEditor(context) {

var graph = context.graph();

function baseDisplayLabel(entity) {
function baseDisplayValue(entity) {
var matched = presetManager.match(entity, graph);
var presetName = (matched && matched.name()) || t('inspector.relation');
var entityName = utilDisplayName(entity) || '';

return presetName + ' ' + entityName;
}

function baseDisplayLabel(entity) {
var matched = presetManager.match(entity, graph);
var presetName = (matched && matched.name()) || t('inspector.relation');
var entityName = utilDisplayName(entity) || '';

return selection => {
selection
.append('b')
.text(presetName + ' ');
selection
.append('span')
.classed('has-colour', entity.tags.colour)
.style('border-color', entity.tags.colour)
.text(entityName);
};
}

var explicitRelation = q && context.hasEntity(q.toLowerCase());
if (explicitRelation && explicitRelation.type === 'relation' && explicitRelation.id !== entityID) {
// loaded relation is specified explicitly, only show that

result.push({
relation: explicitRelation,
value: baseDisplayLabel(explicitRelation) + ' ' + explicitRelation.id
value: baseDisplayValue(explicitRelation) + ' ' + explicitRelation.id,
display: baseDisplayLabel(explicitRelation)
});
} else {

context.history().intersects(context.map().extent()).forEach(function(entity) {
if (entity.type !== 'relation' || entity.id === entityID) return;

var value = baseDisplayLabel(entity);
var value = baseDisplayValue(entity);
if (q && (value + ' ' + entity.id).toLowerCase().indexOf(q.toLowerCase()) === -1) return;

result.push({ relation: entity, value: value });
result.push({
relation: entity,
value,
display: baseDisplayLabel(entity)
});
});

result.sort(function(a, b) {
Expand Down Expand Up @@ -349,6 +371,8 @@ export function uiSectionRawMembershipEditor(context) {
labelLink
.append('span')
.attr('class', 'member-entity-name')
.classed('has-colour', d => d.relation.tags.colour)
.style('border-color', d => d.relation.tags.colour)
Copy link
Copy Markdown
Collaborator

@1ec5 1ec5 Dec 12, 2022

Choose a reason for hiding this comment

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

Should we do any validation on the colour value? As far as I know, the value should be a hex triplet or CSS color keyword, but several other valid CSS color value syntaxes aren’t valid as tag value.

Copy link
Copy Markdown
Collaborator

@1ec5 1ec5 Dec 12, 2022

Choose a reason for hiding this comment

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

Also, some lines are identified by a combination of colors, which would likely be tagged as a semicolon-delimited list. We could show just the first, which could be ambiguous in some cases, or we could attempt to display a color swatch for each, or a gradient perhaps.

Copy link
Copy Markdown
Contributor

@Zaczero Zaczero Jan 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@1ec5
As to your first question, I believe it is unnecessary. In the vast majority of cases, the values are CSS-compliant, and for the rest, I don't think putting in extra work just to handle those would be beneficial. This could be added down the line.

I misunderstood your question during the first read. I can see how users would start using non-standard color values just to make it 'look nice' in the iD. I do support the idea of putting some additional validation for the colors before rendering them out.

Maybe such regex would suffice: ^(#[0-9a-fA-F]{3}|#[0-9a-fA-F]{6}|[a-zA-Z]{3,})$.
It matches either: short hex triplet, hex triplet, or a word with at least 3 characters.
Or, just check how this has been done in the already-existing color field.

To your second question, it is a cool idea that is definitely worth noting as an enhancement issue, but I don't think this lack of support should block this PR. It is non-critical, just a nice-to-have. Anyway, I wrote a small code PoC:

function stripedGradient(colors) {
  var colorArray = colors.split(";");
  var colorStops = "";
  var stopIncrement = 100 / (colorArray.length);
  var currentStop = 0;
  
  for (var i = 0; i < colorArray.length; i++) {
    colorStops += "," + colorArray[i] + " " + currentStop + "%," + colorArray[i] + " " + (currentStop+stopIncrement) + "%";
    currentStop = Math.min(currentStop + stopIncrement + 0.001, 100);
  }
  
  return "linear-gradient(180deg" + colorStops + ")";
}

var cssColors = 'red;blue;black';
var gradient = stripedGradient(cssColors);

image

Copy link
Copy Markdown
Collaborator

@1ec5 1ec5 Jan 24, 2023

Choose a reason for hiding this comment

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

Cool, I agree that handling multiple values could be tail work, though your PoC seems well on its way toward a solution.

Copy link
Copy Markdown
Contributor

@Zaczero Zaczero Jan 25, 2023

Choose a reason for hiding this comment

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

I wonder whether this could be applied to 'normal' color field as well:

image

If this PR gets merged I can make a new one with adding striped colors support to both of them.

Copy link
Copy Markdown
Collaborator

@1ec5 1ec5 Feb 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I misunderstood your question during the first read. I can see how users would start using non-standard color values just to make it 'look nice' in the iD. I do support the idea of putting some additional validation for the colors before rendering them out.

Correct, this was my main feedback. While it would be cool to be able to specify the RGBA or HSLA value of a glass-surfaced bus shelter, I think that would need to start with at least an informal tagging discussion rather than mappers stumbling upon iD’s accidental support for it and latching onto it.

field

member

.text(function(d) { return utilDisplayName(d.relation); });

labelEnter
Expand Down
⚡