Task health pie chart in application list view

[sttts]

[ConnorDoyle]

Pretty slick! @mlunoe, @pyronicide, or @air -- could one of you take a look?

[air]

I took a quick look today - for one thing we need to get the .jshintrc passing on the javascript. For example

• use === (not ==)
• indent with 2 spaces (currently 4)
• 80-char line length

Here's a guide on getting jshint set up with Sublime, https://github.com/mlunoe/react-bb-broccoli#development-setup-sublime-text

[sttts]

Fixed the js style, at least what jsxhint showed me. Let me know if there is anything left.

Where is 4 space indention used?

[air]

I think the latest commit broke PieComponent.jsx, throwing an error on line 68.

If you've verified all indents are 2 spaces we're good on that item.

[sttts]

Fixed. Thanks for reporting. Mixed up Scala and JS syntax...

[sttts]

Rebased to latest master branch.

[apuckey]

Hi there,

Love this PR small issue though. It forever appends new elements until the browser becomes responsive. I fixed this by removing the old element before appending a new one. not sure if this is correct but seems to work :-)

cheers!

[sttts]

Had noticed the ui getting slower over time. Can you attach the patch so I can add it to the pr?

[apuckey]

Sure:

diff --git a/src/main/resources/assets/js/components/PieComponent.jsx b/src/main/resources/assets/js/components/PieComponent.jsx
index 01c6fe4..f9fb6b6 100644
--- a/src/main/resources/assets/js/components/PieComponent.jsx
+++ b/src/main/resources/assets/js/components/PieComponent.jsx
@@ -35,7 +35,10 @@ define([
     _renderGraph:function () {
       var _this = this;
       // Based in http://bl.ocks.org/mbostock/3887235
-      var root = d3.select(this.getDOMNode()).append("g")
+      var node = d3.select(this.getDOMNode());
+      node.selectAll("g").remove();
+
+      var root = node.append("g")
         .attr("transform", "translate(" +
           this.props.width / 2 + "," + this.props.height / 2 + ")"
       );

[mlunoe]

Hi @sttts,

Thank you for putting together all this work! We'd love for this feature to enter marathon, but we do have some requirements to the UI and coding style, so I've put together a couple of comments for you in the pull request. But first here some general comments:

We like simple UI that communicates well. We would love to see this feature implemented, but with a simple stacked progress bar would make it very clear what's going on and that way we don't need the d3.js dependency, which needs some tweaking in order to play well with React.

We rely on bootstrap styles, so you can use their stacked progress bar: http://getbootstrap.com/components/#progress-stacked.

And here's some general styles that should make it a bit slimmer:

.progress {
height: 10px;
border-radius: 3px;
}

.progress-bar {
line-height: 10px;
}
And maybe add a fixed with of 140px: <div className="progress" style={{"width": "140px"}}>

Thank you again!

[mlunoe]

This shouldn't be needed anymore

[sttts]

It was due to rounding errors. In Safari's inspector I saw e.g. 16.66666666668% in case of taskSum=6. Of course this does not add up to 100%.

I added a workaround to cut off digits after the 3rd decimal.

[sttts]

@mlunoe We need a restriction for the black line. Otherwise, we see this:

[sttts]

Rebased and added a workaround for the black-line logic:

CSS seems not to be expressive enough to put a black bar between the colors
in the health-bar, while keeping the 0% colors for correct animation. Hence,
the component has now some code to set the health-bar-inner class which adds
the black line between the colors.

[mlunoe]

Please also fix the jshint/jscs errors reported in the build:

Missing space after "function" keyword at components/AppHealthComponent.js :
    55 |    }, 0);
    56 |
    57 |    var roundWorkaround = function(x) { return Math.floor(x*1000) / 1000; }
------------------------------------------^
    58 |
    59 |    var normalizedHealthData = healthData.map(function (d) {
Missing space before opening round brace at components/AppHealthComponent.js :
    55 |    }, 0);
    56 |
    57 |    var roundWorkaround = function(x) { return Math.floor(x*1000) / 1000; }
------------------------------------------^
    58 |
    59 |    var normalizedHealthData = healthData.map(function (d) {
Operator * should not stick to preceding expression at components/AppHealthComponent.js :
    55 |    }, 0);
    56 |
    57 |    var roundWorkaround = function(x) { return Math.floor(x*1000) / 1000; }
-------------------------------------------------------------------^
    58 |
    59 |    var normalizedHealthData = healthData.map(function (d) {
Operator * should not stick to following expression at components/AppHealthComponent.js :
    55 |    }, 0);
    56 |
    57 |    var roundWorkaround = function(x) { return Math.floor(x*1000) / 1000; }
--------------------------------------------------------------------^
    58 |
    59 |    var normalizedHealthData = healthData.map(function (d) {
If statement without curly braces at components/AppHealthComponent.js :
    69 |    for (var i = 0; i < normalizedHealthData.length; i++) {
    70 |      if (normalizedHealthData[i].width !== "0%") {
    71 |        if (!allZeroWidthBefore)
----------------^
    72 |          normalizedHealthData[i].className += " health-bar-inner";
    73 |        allZeroWidthBefore = false;


components/AppHealthComponent.js: line 57, col 76, Missing semicolon.

1 error

===== 1 JSHint Error

[mlunoe]

Can we make this work on small screens as well with a max-witdh?

width: 100%;
max-width: 140px;

[mlunoe]

Other than that I think this is looking great! Awesome work @sttts!

[sttts]

On small screens:

[mlunoe]

💯 This is an amazing improvement to the UI. Thank you so much for contributing @sttts !!

[sttts]

Cool! Thanks for merging and thanks for all the reviews.

[bobrik]

Awesome! 🎆

[drexin]

This is great stuff! Thanks a lot.

[ConnorDoyle]

👍, major kudos for your perserverance @sttts. Nice result.