Skip to content
This repository has been archived by the owner on Nov 10, 2020. It is now read-only.

Weird FFx/Safari bars bug #1631

Merged
merged 4 commits into from
Jul 21, 2016
Merged

Weird FFx/Safari bars bug #1631

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12fe2af
error breaking national page fixed
gemfarmer Jul 20, 2016
eb3fb3c
filter data that only bars within the appropriate year range render
gemfarmer Jul 20, 2016
b4e9f80
update logic that checks for legend
gemfarmer Jul 20, 2016
7f94aef
remove unnecessary and incorrect conditional
gemfarmer Jul 20, 2016
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: 1 addition & 1 deletion _includes/location/national-exports.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ <h3 class="chart-title">{{ commodity[0] }}</h3>
<eiti-bar-chart
aria-controls="exports-figures-{{ commodity_slug }}"
data='{{ exports | map_hash: _metric | jsonify }}'
data-x-range="{{ year_range }}"
x-range="{{ year_range }}"
x-value="{{ year }}">
</eiti-bar-chart>

Expand Down
2 changes: 1 addition & 1 deletion _includes/location/national-gdp.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ <h3 class="chart-title">GDP ({{ _metric }})</h3>
<eiti-bar-chart
aria-controls="gdp-figures-{{ _metric }}"
data='{{ gdp | map_hash: _metric | jsonify }}'
data-x-range="{{ year_range }}"
x-range="{{ year_range }}"
x-value="{{ year }}">
</eiti-bar-chart>

Expand Down
4 changes: 2 additions & 2 deletions _includes/location/national-jobs.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ <h3 class="chart-title">Wage and salary jobs</h3>
<eiti-bar-chart
aria-controls="jobs-figures-{{ _metric }}"
data='{{ jobs | map_hash: _metric | jsonify }}'
data-x-range="{{ year_range }}"
x-range="{{ year_range }}"
x-value="{{ year }}">
</eiti-bar-chart>
</figure><!-- /.chart -->
Expand Down Expand Up @@ -103,7 +103,7 @@ <h3 class="chart-title">Self-employment jobs</h3>
<eiti-bar-chart
aria-controls="self-employment-figures-{{ _metric }}"
data='{{ self_employment_jobs | map_hash: _metric | jsonify }}'
data-x-range="{{ year_range }}"
x-range="{{ year_range }}"
x-value="{{ year }}">
</eiti-bar-chart>

Expand Down
2 changes: 1 addition & 1 deletion _includes/location/section-exports.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ <h3 class="chart-title">{{ commodity[0] }}</h3>
<eiti-bar-chart
aria-controls="exports-figures-{{ commodity_slug }}"
data='{{ exports | map_hash: _metric | jsonify }}'
data-x-range="{{ year_range }}"
x-range="{{ year_range }}"
x-value="{{ year }}">
</eiti-bar-chart>

Expand Down
2 changes: 1 addition & 1 deletion _includes/location/section-gdp.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ <h3 class="chart-title">GDP ({{ _metric }})</h3>
<eiti-bar-chart
aria-controls="gdp-figures-{{ _metric }}"
data='{{ gdp | map_hash: _metric | jsonify }}'
data-x-range="{{ year_range }}"
x-range="{{ year_range }}"
x-value="{{ year }}">
</eiti-bar-chart>

Expand Down
4 changes: 2 additions & 2 deletions _includes/location/section-jobs.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ <h3 class="chart-title">Wage and salary jobs</h3>
<eiti-bar-chart
aria-controls="jobs-figures-{{ _metric }}"
data='{{ jobs | map_hash: _metric | jsonify }}'
data-x-range="{{ year_range }}"
x-range="{{ year_range }}"
x-value="{{ year }}">
</eiti-bar-chart>
</figure><!-- /.chart -->
Expand Down Expand Up @@ -130,7 +130,7 @@ <h3 class="chart-title">Self-employment</h3>
<eiti-bar-chart
aria-controls="self-employment-figures-{{ _metric }}"
data='{{ self_employment_jobs | map_hash: _metric | jsonify }}'
data-x-range="{{ year_range }}"
x-range="{{ year_range }}"
x-value="{{ year }}">
</eiti-bar-chart>

Expand Down
16 changes: 14 additions & 2 deletions js/components/eiti-bar-chart.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@
.key(function(d) { return d.x; })
.rollup(function(d) { return d[0]; })
.entries(data);


} else {
values = Object.keys(data).reduce(function(map, key) {
map[key] = {x: +key, y: data[key]};
Expand All @@ -81,9 +83,9 @@
});
}

// console.log('data:', data, 'values:', values);

var xrange = this.xrange;

if (!xrange) {
xrange = d3.extent(data, function(d) {
return +d.x;
Expand All @@ -102,6 +104,12 @@
}
});

// filter the data so that only values within the domain
// are included in calculations and rendering
data = data.filter(function(d){
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a space between the ) and {. I'm wondering why Hound didn't spot this...

return xdomain.indexOf(d.x) > -1;
});

var extent = d3.extent(data, function(d) { return d.y; });
var ymax = extent[1];
var ymin = Math.min(0, extent[0]);
Expand Down Expand Up @@ -136,7 +144,11 @@
bars.exit().remove();

bars.attr('transform', function(d) {
return 'translate(' + [x(d.x), 0] + ')';
if (x(d.x)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this this right test? Presumably x(d.x) could evaluate to zero, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe ?

if (x(d.x) && x(d.x) !== 0) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me play around with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, it looks like this was fixed by the filter function above 😄

return 'translate(' + [x(d.x), 0] + ')';
} else {
console.warn(d.x, 'not in bar chart domain')
}
});

bars.select('.bar-value')
Expand Down
94 changes: 46 additions & 48 deletions js/components/eiti-data-map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,46 +87,52 @@
var svgLegend = d3.select(this)
.select('.legend-svg');

svgLegend.append('g')
.attr('class', 'legendScale');

var legend = d3.legend.color()
.labelFormat(format.si)
.useClass(false)
.ascending(true)
.labelDelimiter('-')
.shapePadding(6)
.scale(scale);

svgLegend.select('.legendScale')
.call(legend);

// reverse because the scale is in ascending order
var _steps = d3.range(0, 9).reverse();

// find which steps are represented in the map
var uniqueSteps = getUnique(marks.data(), _steps, domain);

// start consolidate (translate) visible cells
var cells = svgLegend.selectAll('.cell');
var cellHeight = legend.shapeHeight() + legend.shapePadding();
var count = 0;
cells.each(function(cell, i) {
var present = uniqueSteps.indexOf(i) > -1;

if (!present) {
// hide cells swatches that aren't in the map
cells[0][i].setAttribute('aria-hidden', true);
count++;
} else {
// trim spacing between swatches that are visible
var translateHeight = (i * cellHeight) - (count * cellHeight);
cells[0][i].setAttribute('transform',
'translate(0,' + translateHeight + ')');
}
});
// end consolidation
// end map legend
if (svgLegend[0][0]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

More succinctly: !svgLegend.empty()

svgLegend.append('g')
.attr('class', 'legendScale');

var legend = d3.legend.color()
.labelFormat(format.si)
.useClass(false)
.ascending(true)
.labelDelimiter('-')
.shapePadding(6)
.scale(scale);

svgLegend.select('.legendScale')
.call(legend);

// reverse because the scale is in ascending order
var _steps = d3.range(0, 9).reverse();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker, but why _steps here and not just steps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I did this to not overwrite steps from above, but I guess I'm not totally sure that would be problematic, come to think of it. I merged for now, because this came up in standup, but should be refactored later.


// find which steps are represented in the map
var uniqueSteps = getUnique(marks.data(), _steps, domain);

// start consolidate (translate) visible cells
var cells = svgLegend.selectAll('.cell');
var cellHeight = legend.shapeHeight() + legend.shapePadding();
var count = 0;
cells.each(function(cell, i) {
var present = uniqueSteps.indexOf(i) > -1;

if (!present) {
// hide cells swatches that aren't in the map
cells[0][i].setAttribute('aria-hidden', true);
count++;
} else {
// trim spacing between swatches that are visible
var translateHeight = (i * cellHeight) - (count * cellHeight);
cells[0][i].setAttribute('transform',
'translate(0,' + translateHeight + ')');
}
});
// end consolidation
// end map legend
} else {
console.warn('<eiti-data-map> does not exist on this page.')
Copy link
Contributor

Choose a reason for hiding this comment

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

This should say that the legend doesn't exist, right?

Copy link
Contributor Author

@gemfarmer gemfarmer Jul 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

how about this <eiti-data-map> element does not have an associated svg legend.?

}



// start trim height on map container
var svgContainer = d3.select(this)
Expand All @@ -145,14 +151,6 @@
svgContainer.style('padding-bottom', pixelize);
// end trim


// var swatches = d3.select(this).selectAll('.swatch[data-value-swatch]')
// .datum(function() {
// return +this.getAttribute('data-value-swatch') || 0;
// });

// swatches.style('background-color', scale);
// console.log(swatches)
}}
}
)
Expand Down
110 changes: 60 additions & 50 deletions js/lib/state-pages.min.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11177,46 +11177,52 @@
var svgLegend = d3.select(this)
.select('.legend-svg');

svgLegend.append('g')
.attr('class', 'legendScale');

var legend = d3.legend.color()
.labelFormat(format.si)
.useClass(false)
.ascending(true)
.labelDelimiter('-')
.shapePadding(6)
.scale(scale);

svgLegend.select('.legendScale')
.call(legend);

// reverse because the scale is in ascending order
var _steps = d3.range(0, 9).reverse();

// find which steps are represented in the map
var uniqueSteps = getUnique(marks.data(), _steps, domain);

// start consolidate (translate) visible cells
var cells = svgLegend.selectAll('.cell');
var cellHeight = legend.shapeHeight() + legend.shapePadding();
var count = 0;
cells.each(function(cell, i) {
var present = uniqueSteps.indexOf(i) > -1;

if (!present) {
// hide cells swatches that aren't in the map
cells[0][i].setAttribute('aria-hidden', true);
count++;
} else {
// trim spacing between swatches that are visible
var translateHeight = (i * cellHeight) - (count * cellHeight);
cells[0][i].setAttribute('transform',
'translate(0,' + translateHeight + ')');
}
});
// end consolidation
// end map legend
if (svgLegend[0][0]) {
svgLegend.append('g')
.attr('class', 'legendScale');

var legend = d3.legend.color()
.labelFormat(format.si)
.useClass(false)
.ascending(true)
.labelDelimiter('-')
.shapePadding(6)
.scale(scale);

svgLegend.select('.legendScale')
.call(legend);

// reverse because the scale is in ascending order
var _steps = d3.range(0, 9).reverse();

// find which steps are represented in the map
var uniqueSteps = getUnique(marks.data(), _steps, domain);

// start consolidate (translate) visible cells
var cells = svgLegend.selectAll('.cell');
var cellHeight = legend.shapeHeight() + legend.shapePadding();
var count = 0;
cells.each(function(cell, i) {
var present = uniqueSteps.indexOf(i) > -1;

if (!present) {
// hide cells swatches that aren't in the map
cells[0][i].setAttribute('aria-hidden', true);
count++;
} else {
// trim spacing between swatches that are visible
var translateHeight = (i * cellHeight) - (count * cellHeight);
cells[0][i].setAttribute('transform',
'translate(0,' + translateHeight + ')');
}
});
// end consolidation
// end map legend
} else {
console.warn('<eiti-data-map> does not exist on this page.')
}



// start trim height on map container
var svgContainer = d3.select(this)
Expand All @@ -11235,14 +11241,6 @@
svgContainer.style('padding-bottom', pixelize);
// end trim


// var swatches = d3.select(this).selectAll('.swatch[data-value-swatch]')
// .datum(function() {
// return +this.getAttribute('data-value-swatch') || 0;
// });

// swatches.style('background-color', scale);
// console.log(swatches)
}}
}
)
Expand Down Expand Up @@ -12393,6 +12391,8 @@
.key(function(d) { return d.x; })
.rollup(function(d) { return d[0]; })
.entries(data);


} else {
values = Object.keys(data).reduce(function(map, key) {
map[key] = {x: +key, y: data[key]};
Expand All @@ -12403,9 +12403,9 @@
});
}

// console.log('data:', data, 'values:', values);

var xrange = this.xrange;

if (!xrange) {
xrange = d3.extent(data, function(d) {
return +d.x;
Expand All @@ -12424,6 +12424,12 @@
}
});

// filter the data so that only values within the domain
// are included in calculations and rendering
data = data.filter(function(d){
return xdomain.indexOf(d.x) > -1;
});

var extent = d3.extent(data, function(d) { return d.y; });
var ymax = extent[1];
var ymin = Math.min(0, extent[0]);
Expand Down Expand Up @@ -12458,7 +12464,11 @@
bars.exit().remove();

bars.attr('transform', function(d) {
return 'translate(' + [x(d.x), 0] + ')';
if (x(d.x)) {
return 'translate(' + [x(d.x), 0] + ')';
} else {
console.warn(d.x, 'not in bar chart domain')
}
});

bars.select('.bar-value')
Expand Down