-
Notifications
You must be signed in to change notification settings - Fork 2
Barchart/marker #2
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
base: main
Are you sure you want to change the base?
Barchart/marker #2
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a lot of indentation, and unnecessary enters in the changed files, to fix all the style issues, maybe consider using npx prettier --write "<your file path>/**/*.{ts,tsx,js,jsx}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! You really managed to get something nice, even though the surrounding code is a bit of a mess. I haven't managed to get Grafana to start yet on my machine, but the code looks good already! I will attempt again to get it to run. Most of my suggestions are cosmetic or small things.
| return { barData, markerData }; | ||
| } | ||
|
|
||
| export function deepCopy<T>(obj: T, seen = new Map<any, any>()): T { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must already exist as a library function somewhere right? I would search for that. I can't imagine this is the first time it's needed.
| max={2} | ||
| step={0.01} | ||
| value={marker.opts.size ?? 1} | ||
| onChange={(v) => handleOptsSettingChange(marker.id, 'size', typeof v === 'number' ? v : v[0])} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v will always be a number according to my type checker, so you don't need to do the typeof check
Co-authored-by: Lars Stegman <LarsStegman@users.noreply.github.com>
What is this feature?
This feature allows the user to add markers to a barchart based on an input field.
Why do we need this feature?
This feature allows the user to visualize a threshold
Who is this feature for?
[Add information on what kind of user the feature is for.]
Which issue(s) does this PR fix?:
**Special notes for your reviewer:**x
Please check that: