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

Genericify refactor #1

Merged
merged 12 commits into from
Jan 26, 2024
Merged

Genericify refactor #1

merged 12 commits into from
Jan 26, 2024

Conversation

jsbnr
Copy link
Contributor

@jsbnr jsbnr commented Jan 16, 2024

DO NOT MERGE NOT COMPLETE!

This PR contains a refactor of some of the configuration and functionality moving it from hard coded config/javascript to config options with the intention to make it easier to deploy and use across multiple dashboards.

Main changes include:

  • Query is now configurable as a viz option
  • Disable zoom cluserting now a config option
  • Center point of map can be specified as lat/lng coords, removed list
  • Some naming updates to be less 'store' specific
  • Changed how alert thresholds work, now powered by values defined in query (threshold_critical, threshold_warning), can also now work in both directions as required
  • Removed hard coded required query result values and replaced with conventions (value, name, etc)
  • Changed tooltip to appear on hover instead of click
  • Made the tooltip content configurable via query values named convention tooltip_xxx
  • Changed cluster colour to neutral grey instead of green.
  • Made marker displayed value configurable via nrql value (icon_label)
  • Added config option for refresh interval
  • Changed the empty state to be more useful
  • Fixed zero location result causing oops
  • Added option to provide link in NRQL results which can be navigated by clicking on a marker

TODO:

  • Review with Mat
  • Decide what to do with regions

Copy link
Contributor

@matewilk matewilk left a comment

Choose a reason for hiding this comment

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

Looks amazing, just a few minor comments.

visualizations/store-map-viz/components/EmptyState.tsx Outdated Show resolved Hide resolved
visualizations/store-map-viz/constants.ts Outdated Show resolved Hide resolved
visualizations/store-map-viz/styles.scss Outdated Show resolved Hide resolved
@jsbnr
Copy link
Contributor Author

jsbnr commented Jan 22, 2024

Decision: to remove regions from the app, these can be added as a new feature so the code has been left in place but is not used.

@matewilk matewilk merged commit 7ba3d8d into main Jan 26, 2024
3 checks passed
Copy link
Contributor

@matewilk matewilk left a comment

Choose a reason for hiding this comment

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

Looks good!

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.

2 participants