Skip to content
This repository was archived by the owner on Nov 14, 2023. It is now read-only.

Conversation

@cseas
Copy link

@cseas cseas commented Dec 7, 2022

Description

Jira | Stage environment

Before
Screenshot 2022-12-12 at 3 35 27 PM
After
Screenshot 2022-12-12 at 3 37 22 PM

Testing

  • Metric widgets on home screen work
  • Metric widgets on service overview screen work
  • Metric widget: Interval = None, GroupBy = None
  • Cartesian widget: Interval = Defined, GroupBy = None
  • Cartesian widget: Interval = None, GroupBy = Defined
  • Cartesian widget: Interval = Defined, GroupBy = Defined

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

@github-actions

This comment has been minimized.

@codecov
Copy link

codecov bot commented Dec 7, 2022

Codecov Report

Merging #107 (3455d7c) into rzp_main (55c6cc3) will decrease coverage by 2.08%.
The diff coverage is 53.57%.

@@             Coverage Diff              @@
##           rzp_main     #107      +/-   ##
============================================
- Coverage     81.91%   79.82%   -2.09%     
============================================
  Files           931      933       +2     
  Lines         20013    20133     +120     
  Branches       2708     2740      +32     
============================================
- Hits          16393    16071     -322     
- Misses         3472     3891     +419     
- Partials        148      171      +23     
Impacted Files Coverage Δ
...a/graphql/entity/entity-value-data-source.model.ts 91.66% <ø> (ø)
...explorer-visualization-metric-data-source.model.ts 23.80% <23.80%> (ø)
...gets/metric-display/metric-display-widget.model.ts 35.84% <33.33%> (-49.26%) ⬇️
...hql/explore/explore-cartesian-data-source.model.ts 89.55% <66.66%> (-1.22%) ⬇️
...rc/pages/explorer/utils/get-layout-for-elements.ts 81.81% <81.81%> (ø)
...y/src/pages/explorer/explorer-dashboard-builder.ts 89.41% <90.90%> (+0.22%) ⬆️
...lorer-visualization-cartesian-data-source.model.ts 84.21% <100.00%> (ø)
src/app/home/home.module.ts 0.00% <0.00%> (-100.00%) ⬇️
src/app/home/home.component.ts 0.00% <0.00%> (-100.00%) ⬇️
src/app/home/home.dashboard.ts 0.00% <0.00%> (-100.00%) ⬇️
... and 67 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@cseas cseas added ReadyForReview ReadyForReview and removed DoNotReview labels Dec 12, 2022
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@cseas cseas requested a review from jaywalker21 December 13, 2022 03:44
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@cseas cseas requested a review from amandal97 December 13, 2022 03:59
Copy link

@jaywalker21 jaywalker21 left a comment

Choose a reason for hiding this comment

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

Few comms

this.requestSubject.next(request);
}

private getLayoutForElements(numberOfElements: number): object {

Choose a reason for hiding this comment

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

Do you think we can automate this?
Also, if not automate, reuse the other parts apart from the cell-span thing?

Copy link
Author

@cseas cseas Dec 13, 2022

Choose a reason for hiding this comment

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

Added a function to generate column dimensions since they're all the same.

Not sure if one for row dimensions is possible since rows are unequal for less than 5 elements and equal afterwards.

For cell spans, can probably write a function to decrease lines of code but it'll get harder to read and modify code later. Should I add it?

Choose a reason for hiding this comment

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

I think for rows, we are showing 4 items per row right? If that value is static, the row thing can be automated too. If you're disabling the option to add more values beyond a certain count, I am fine with things being static in code.

Copy link
Author

@cseas cseas Dec 14, 2022

Choose a reason for hiding this comment

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

Not always, here's the layout:

No. of metrics Grid used Position of items Row height
1 3 x 3 Single item in the middle cell, other cells empty middle row is bigger
2 to 4 3 x 4 4 items in the middle row, top & bottom row empty middle row is bigger
5 to 8 2 x 4 4 items each in two rows, full grid filled rows are equal height

All layouts have different combination of rows & columns. Not sure what's the cap we should have for number of metrics. It's not a limitation really, I can keep adding more grid layouts but do we know any teams using panels with more than 8 metrics?

The chart height also can be changed depending on how many metrics we want to fit. Like if someone really wants 30 metrics in a panel, the height can be increased to fit more but then time series charts will also become bigger permanently.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@cseas cseas merged commit 34afe6b into rzp_main Dec 19, 2022
@cseas cseas deleted the explorer-metric-widget branch December 19, 2022 06:45
@github-actions
Copy link

Unit Test Results

       4 files  ±0     303 suites  +1   24m 52s ⏱️ +44s
1 087 tests +3  1 085 ✔️ +1  0 💤 ±0  2 ❌ +2 
1 095 runs  +3  1 093 ✔️ +1  0 💤 ±0  2 ❌ +2 

For more details on these failures, see this check.

Results for commit 34afe6b. ± Comparison against base commit 55c6cc3.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ReadyForReview ReadyForReview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants