Counts summary section on overview #1635
Conversation
|
|
||
| const Summary = ({ data }) => ( | ||
| <Styled> | ||
| {data.map(el => <GaugeChart value={el.winPercent} caption={el.category} />)} |
There was a problem hiding this comment.
It's not very clear these are winrates, they look more like proportions. I wonder if there's a good way to show both?
There was a problem hiding this comment.
I changed the styling. Hopefully now it's more clear.
| /> | ||
| </Container> | ||
| </HeroesContainer> | ||
| <Container |
There was a problem hiding this comment.
I think this would look better above the matches/heroes tables
| props.getPlayerRecentMatches(props.playerId); | ||
| props.getPlayerHeroes(props.playerId, props.location.search); | ||
| props.getPlayerPeers(props.playerId, props.location.search); | ||
| props.getPlayerCounts(props.playerId, props.location.query); |
There was a problem hiding this comment.
why does this use .query instead of .search?
|
|
||
| const limitCount = (key, field, lim) => | ||
| counts[key].list.sort((a, b) => ( | ||
| a[field] <= b[field] ? 1 : -1 |
There was a problem hiding this comment.
can you just do b[field] - a[field]?
| tower_damage: [], | ||
| duration: [], | ||
| wins: [], | ||
| game_mode: [], |
There was a problem hiding this comment.
I don't think this makes sense to add to the summary?
|
|
||
| Object.keys(counts).forEach((key) => { | ||
| switch (key) { | ||
| case 'is_radiant': |
There was a problem hiding this comment.
maybe we should show the top two for each, and put each in its own column? not sure if that would make it too tall.
There was a problem hiding this comment.
I think it would be too tall.
There was a problem hiding this comment.
Think we could still do top two for each but leave it one row. That would be 8 gauges (4x2)
There was a problem hiding this comment.
could possibly also fit one more set of gauges, maybe the top two lane_role counts (skipping unknown)
| countsError, | ||
| }) => ( | ||
| <OverviewContainer> | ||
| <SummaryContainer |
There was a problem hiding this comment.
thinking we should add the rounded border to the summary component as well. they should look similar.
|
I didn't localize "Win" and "Loss" because we cannot guarantee that the strings fit in any language. |
|
I reverted the colors. The green/red was way too colorful/bright. |
|
|
||
| const GaugeChart = ({ value, caption }) => ( | ||
| <Styled percent={computeMeterPercent(value)} meterColor={computeMeterColor(value)}> | ||
| <div className="caption">{caption}</div> |
There was a problem hiding this comment.
I think we could display the number of matches as well, maybe as parens after the category, like DIRE (1020)
| </div> | ||
| </div> | ||
| </div> | ||
| <div className="win">Win</div> |
There was a problem hiding this comment.
maybe W and L to make it more general instead of explicitly English?
|
Fixed. It's really easy to read at a quick glance but also dense in information now. |
|
I wonder if we could put in some kind of separator element between the gauges for each type of count. Aside from that, I like this a lot! |
|
-added a small visual divider edit: I actually made them more obvious now |
fixes #771