docs: Fix Flux CSV README examples #761
Conversation
- Add required LayerConfig `fill` property. - Revise examples.
| ```ts | ||
| import { fromFlux, Plot, LayerTypes, Config, LayerConfig } from '@influxdata/giraffe' | ||
|
|
||
| export default function DevicePlot() { |
There was a problem hiding this comment.
minor nitpick, but I don't think we should encourage export default in docs for some of the reasons outlined here: https://basarat.gitbook.io/typescript/main-1/defaultisbad.
It would look like this when removing the export default:
export const DevicePlot = () => {| table: dataFromFlux.table, | ||
| layers: [lineLayer], | ||
| } | ||
|
|
||
| // ... | ||
| return( |
There was a problem hiding this comment.
| return( | |
| return ( |
|
|
||
| <pre> | ||
| import {Plot} from '@influxdata/giraffe' | ||
| export default function DevicePlot() { |
There was a problem hiding this comment.
same bit here about export default
export const DevicePlot = () => {| <pre> | ||
| import {Plot, fromFlux} from '@influxdata/giraffe' | ||
| ```ts | ||
| import { fromFlux, Plot, LayerTypes, Config, LayerConfig } from '@influxdata/giraffe' |
There was a problem hiding this comment.
Let's alphabetize these imports, and remove the spaces between the brackets, since that is our coding standard.
import {fromFlux, Config, LayerConfig, LayerTypes, Plot}|
|
||
| When using the comma separated values (CSV) from the Flux query as the `fluxResponse` property: | ||
| ```ts | ||
| import { Plot, LayerTypes, Config, LayerConfig } from '@influxdata/giraffe' |
There was a problem hiding this comment.
Same here about alphabetizing and removing the spaces around the braces
import {Config, LayerConfig, LayerTypes, Plot}
There was a problem hiding this comment.
We should avoid TypeScript in the examples. I find TypeScript to be an advanced tool. Our examples should be approachable. Someone who wants to use just React does not need to import LayerTypes.
| fluxResponse, | ||
| layers: [lineLayer], | ||
| } | ||
|
|
||
| // ... | ||
| return( |
There was a problem hiding this comment.
our standards have a space between return and ( or {
| return( | |
| return ( |
|
|
||
| When using the comma separated values (CSV) from the Flux query as the `fluxResponse` property: | ||
| ```ts | ||
| import { Plot, LayerTypes, Config, LayerConfig } from '@influxdata/giraffe' |
There was a problem hiding this comment.
We should avoid TypeScript in the examples. I find TypeScript to be an advanced tool. Our examples should be approachable. Someone who wants to use just React does not need to import LayerTypes.
| const lineLayer = { | ||
| type: "line", | ||
| const lineLayer: LayerConfig = { | ||
| type: LayerTypes.Line, |
There was a problem hiding this comment.
We should avoid TypeScript in the examples. I find TypeScript to be an advanced tool. Our examples should be approachable. Someone who wants to use just React needs only the string "line"
| } | ||
|
|
||
| const config = { | ||
| const config: Config = { |
There was a problem hiding this comment.
We should avoid TypeScript in the examples. I find TypeScript to be an advanced tool. Our examples should be approachable. No need to confuse someone who just wants (and knows only how) to use React.
fillproperty.