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.

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

Show table of cell health state changes instead of chart #3135

Merged
merged 6 commits into from
Nov 1, 2018

Conversation

richard-cox
Copy link
Contributor

  • fixes Improve Cell Health visual #3127

  • Table shows changes to cell health

  • To test more populated table make the following change to src/frontend/app/shared/components/list/list-types/cf-cell-health/cf-cell-health-source.ts

    const values = response[0].data.result[0].values;
    

    to

    const values = response[0].data.result[0].values.map(value => {
      const timestamp = value['0'];
      return [
        timestamp,
        timestamp * 1000 % 2 === 0 ? '1' : '0'
      ];
    });
    

- Table shows changes to cell health
- To test more populated table make the following change to `src/frontend/app/shared/components/list/list-types/cf-cell-health/cf-cell-health-source.ts`

const values = response[0].data.result[0].values;

to

const values = response[0].data.result[0].values.map(value => {
  const timestamp = value['0'];
  return [
    timestamp,
    timestamp * 1000 % 2 === 0 ? '1' : '0'
  ];
});
@cfdreddbot
Copy link

Hey richard-cox!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@codecov
Copy link

codecov bot commented Oct 17, 2018

Codecov Report

Merging #3135 into v2-master will increase coverage by 24.8%.
The diff coverage is 77.95%.

@@              Coverage Diff              @@
##           v2-master    #3135      +/-   ##
=============================================
+ Coverage      46.31%   71.11%   +24.8%     
=============================================
  Files            630      632       +2     
  Lines          27580    27683     +103     
  Branches        6271     6297      +26     
=============================================
+ Hits           12773    19687    +6914     
+ Misses         14807     7996    -6811

@KlapTrap KlapTrap self-requested a review October 31, 2018 09:54
Copy link
Contributor

@KlapTrap KlapTrap left a comment

Choose a reason for hiding this comment

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

Clicking the refresh button causes the table to become empty.

If you select a custom date range that does have any data then an exception is thrown.

@richard-cox
Copy link
Contributor Author

@KlapTrap I've fixed the bugs and created #3162 to improve the 'one record' case

@KlapTrap
Copy link
Contributor

I'm a little un-easy about having app-metrics-range-selector within the list component, it feels like we're overloading the list component a bit. I would have preferred the list component be a child of the parent range selector.

If we decided to create a separate component for #3162 then we should ensure we should remove the app-metrics-range-selector

@KlapTrap
Copy link
Contributor

I'm happy to merge this once the gate has passed.

@KlapTrap KlapTrap merged commit 117aadd into v2-master Nov 1, 2018
@KlapTrap KlapTrap deleted the cell-health-tweak-2 branch November 1, 2018 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Cell Health visual
3 participants