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

Refactor shield functions: #274

Open
wants to merge 6 commits into
base: gh-pages
Choose a base branch
from
Open

Refactor shield functions: #274

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f283337
Refactor sprite functions:
hjanetzek Dec 8, 2018
ccf6081
Add back special 4-5 char priority filter
hjanetzek Dec 8, 2018
a11dfe7
Move visible: global.sdk_road_shields to overall 'shields' layer
hjanetzek Dec 8, 2018
1b7e6fb
global.ux_language_text_source formatting and cleanups
hjanetzek Dec 8, 2018
cd16294
Fix duplicate US-US:
hjanetzek Dec 8, 2018
3d56897
Unset 'generic' sprite fallback for shields
hjanetzek Dec 8, 2018
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
153 changes: 98 additions & 55 deletions bubble-wrap-style.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,57 +14,95 @@ global:
#ux/ui
ux_language: false # l10n language code, trusting OSM in v0.10 tiles, fixed in v1.0 tiles
ux_language_fallback: false # l10n language code, trusting OSM in v0.10 tiles, fixed in v1.0 tiles
# if a ux_langauge has been defined use that, else if there is feature name
Copy link
Member

Choose a reason for hiding this comment

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

Nit: add line space above here as it's getting a little crowded to read now.

# in the fallback_ux_language then use that, else use the feature's default
# name in the local language
ux_language_text_source: |
function() {
// if a ux_langauge has been defined use that, else if there is feature name in the fallback_ux_language then use that, else use the feature's default name in the local language
return (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature.name;
return (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) ||
feature.name;
}
ux_language_text_source_left: |
function() {
// if a ux_langauge has been defined use that, else if there is feature name in the fallback_ux_language then use that, else use the feature's default name in the local language
return (global.ux_language && feature['name:left:'+global.ux_language]) || (global.ux_language_fallback && feature['name:left:'+global.ux_language_fallback]) || feature['name:left'];
return (global.ux_language && feature['name:left:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:left:'+global.ux_language_fallback]) ||
feature['name:left'];
}
ux_language_text_source_right: |
function() {
// if a ux_langauge has been defined use that, else if there is feature name in the fallback_ux_language then use that, else use the feature's default name in the local language
return (global.ux_language && feature['name:right:'+global.ux_language]) || (global.ux_language_fallback && feature['name:right:'+global.ux_language_fallback]) || feature['name:right'];
return (global.ux_language && feature['name:right:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:right:'+global.ux_language_fallback]) ||
feature['name:right'];
}
ux_language_text_source_boundary_lines_left_right:
left: global.ux_language_text_source_left
right: global.ux_language_text_source_right
ux_language_text_source_boundary_lines: |
function() {
var right = (global.ux_language && feature['name:right:'+global.ux_language]) || (global.ux_language_fallback && feature['name:right:'+global.ux_language_fallback]) || feature['name:right'];
var left = (global.ux_language && feature['name:left:'+global.ux_language]) || (global.ux_language_fallback && feature['name:left:'+global.ux_language_fallback]) || feature['name:left'];
var right = (global.ux_language && feature['name:right:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:right:'+global.ux_language_fallback]) ||
feature['name:right'];
var left = (global.ux_language && feature['name:left:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:left:'+global.ux_language_fallback]) ||
feature['name:left'];
if( right && left ) {
//if( right.includes(' ') || left.includes(' ') ) {
return left + " - " + right;
//} else {
// return right + '\n' + left;
//}
return left + " - " + right;
} else {
return (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature.name;
return (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature.name;
}
}
# ux_language_text_source_boundary_lines: |
# function() {
# var right = (global.ux_language && feature['name:right:'+global.ux_language]) ||
# (global.ux_language_fallback && feature['name:right:'+global.ux_language_fallback]) ||
# feature['name:right'];
# var left = (global.ux_language && feature['name:left:'+global.ux_language]) ||
# (global.ux_language_fallback && feature['name:left:'+global.ux_language_fallback]) ||
# feature['name:left'];
# if( right && left ) {
# if( right.includes(' ') || left.includes(' ') ) {
Copy link
Member

Choose a reason for hiding this comment

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

Removing this if/else logic for stacking or not stacking the resulting ux_language_text_source_boundary_lines text string would be a visual regression.

# return left + " - " + right;
# } else {
# return right + '\n' + left;
# }
# } else {
# return (global.ux_language && feature['name:'+global.ux_language]) ||
# (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature.name;
# }
# }
ux_language_text_source_short: |
function() {
return (global.ux_language && feature['name:short:'+global.ux_language]) || (global.ux_language_fallback && feature['name:short:'+global.ux_language_fallback]) || feature['name:short'];
return (global.ux_language && feature['name:short:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:short:'+global.ux_language_fallback]) ||
feature['name:short'];
}
ux_language_text_source_short_proxy_name: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'] || '';
var short = (global.ux_language && feature['name:short:'+global.ux_language]) || (global.ux_language_fallback && feature['name:short:'+global.ux_language_fallback]) || feature['name:short'];
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) ||
feature['name'] || '';
var short = (global.ux_language && feature['name:short:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:short:'+global.ux_language_fallback]) ||
feature['name:short'];
return short ? name : '';
}
ux_language_text_source_abbreviation: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'] || '';
var abbrev = (global.ux_language && feature['name:abbreviation:'+global.ux_language]) || (global.ux_language_fallback && feature['name:abbreviation:'+global.ux_language_fallback]) || feature['name:abbreviation'];
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) ||
feature['name'] || '';
var abbrev = (global.ux_language && feature['name:abbreviation:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:abbreviation:'+global.ux_language_fallback]) ||
feature['name:abbreviation'];
return abbrev || name;
}
ux_language_text_source_iata: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) ||
feature['name']
if(feature.iata) {
if (name) {
return name + ' (' + feature.iata + ')';
Expand All @@ -78,68 +116,73 @@ global:
}
ux_language_text_source_ocean: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'] || '';
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) ||
feature['name'] || '';
name = name.split(' ').join('\n');
return name.split('').join(' ');
}
ux_language_text_source_sea: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'] || '';
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) ||
feature['name'] || '';
name = name.split(' ').join('\n');
return name.split('').join(' ');
}
ux_language_text_source_continent_stacked_only: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'] || '';
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) ||
feature['name'] || '';
return name.split(' ').join('\n');
}
ux_language_text_source_continent: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'] || '';
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) ||
feature['name'] || '';
name = name.split(' ').join('\n');
return name.split('').join(' ');
}
ux_language_text_source_road_ref_and_name: |
function() {
// if a ux_langauge has been defined use that, else if there is feature name in the fallback_ux_language then use that, else use the feature's default name in the local language
return (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature.name;

/*
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
if(feature.ref && name) {
return (feature.ref + ' ' + name);
} else {
return name;
}
*/
}
ux_language_text_source_road_ref_and_name_short: |
function() {
// if a ux_langauge has been defined use that, else if there is feature name in the fallback_ux_language then use that, else use the feature's default name in the local language
return (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature.name;

/*
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
if (feature.ref && (feature.ref.length < 6) && name) {
return feature.ref + ' ' + name;
} else {
return name;
}
*/
}
ux_language_text_source_road_ref_and_name: global.ux_language_text_source
Copy link
Member

Choose a reason for hiding this comment

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

Definitely better to just reuse the existing function like this where possible, but this also reminded me, I added a simple cache to JS recently to reuse scene JS functions internally when they have the same source (similar to caching shader programs by source), and it can help catch cases like this.

Copy link
Member

Choose a reason for hiding this comment

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

Seems worth a separate Tangram ES issue... @hjanetzek @matteblair?

Copy link
Member Author

Choose a reason for hiding this comment

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

We compile identical functions only one time (per js context) as well :)

# ux_language_text_source_road_ref_and_name: |
Copy link
Member

Choose a reason for hiding this comment

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

I'm okay removing the commented out ux_language_text_source_road_ref_and_name section – it's vestigial from when Tangram didn't really support road shields.

# function() {
# var name = (global.ux_language && feature['name:'+global.ux_language]) ||
# (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
# if(feature.ref && name) {
# return (feature.ref + ' ' + name);
# } else {
# return name;
# }
# }
ux_language_text_source_road_ref_and_name_short: global.ux_language_text_source
# ux_language_text_source_road_ref_and_name_short: |
Copy link
Member

Choose a reason for hiding this comment

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

I'm okay removing the commented out ux_language_text_source_road_ref_and_name_short section – it's vestigial from when Tangram didn't really support road shields.

# function() {
# var name = (global.ux_language && feature['name:'+global.ux_language]) ||
# (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
# if (feature.ref && (feature.ref.length < 6) && name) {
# return feature.ref + ' ' + name;
# } else {
# return name;
# }
# }
ux_language_text_source_piste_advanced: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
Copy link
Member

Choose a reason for hiding this comment

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

nit: The convention above in this PR is to separate each || onto a new line, I think feature['name']; here as well?

return name ? ('◆ ' + name) : '◆';
}
ux_language_text_source_piste_expert: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
Copy link
Member

Choose a reason for hiding this comment

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

nit: The convention above in this PR is to separate each || onto a new line, I think feature['name']; here as well?

return name ? ('◆◆ ' + name) : '◆◆';
}
ux_language_text_source_building_and_address: |
function() {
var name = (global.ux_language && feature['name:'+global.ux_language]) || (global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
var name = (global.ux_language && feature['name:'+global.ux_language]) ||
(global.ux_language_fallback && feature['name:'+global.ux_language_fallback]) || feature['name'];
Copy link
Member

Choose a reason for hiding this comment

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

nit: The convention above in this PR is to separate each || onto a new line, I think feature['name']; here as well?

if (name && feature.addr_housenumber) {
return name + '\n' + feature.addr_housenumber;
} else {
Expand Down