Skip to content

Conversation

@alannahw
Copy link
Contributor

@alannahw alannahw commented Nov 1, 2017

  • to display the top five offenders from the usage distribution
  • made arc responsive to hover for visibility
  • moved metrics number from under sunburst to center of diagram

* to display the top five offenders from the usage distribution
* made arc responsive to hover for visibility
* moved metrics number from under sunburst to center of diagram
@davkal
Copy link
Contributor

davkal commented Nov 2, 2017

The code to the generate the tables is great.

The CSS needs adapting though: This project uses basscss classes to express styles, and only does minimal adjustments with local classes (ideally there would be zero). The goal is maximum reusability in cases where sub-components are included into other projects. The fewer explicit styles are in this repo, the more freedom the super-project has in laying things out.

Here is a screenshot on how the table looks when included in a super-project:

screen shot 2017-11-02 at 15 29 09

Concretely, I would ask you to look through the CSS changes of this PR and see if they can also be done via bass classes. Let's take it from there.

Last thing: there needs to be a separation between the label and the metric name in each table row.

* level names separated for legibility
* local css styling changed to basscss styling for consistency
@davkal davkal self-requested a review November 6, 2017 12:42
Copy link
Contributor

@davkal davkal left a comment

Choose a reason for hiding this comment

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

Nice table!

@davkal davkal merged commit 68af503 into grafana:master Nov 6, 2017
@rgeyer rgeyer mentioned this pull request Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants