WIP: Line Graph Component #21

Closed
b72zhou wants to merge 14 commits from b72zhou-line-graph into main
Collaborator

Things to be fixed:

  • Line is not aligned with x-axis

Todo:

  • Hover effects

  • Set colors with global variables

Things to be fixed: - [x] Line is not aligned with x-axis Todo: - [ ] Hover effects - [x] Set colors with global variables
b72zhou added 1 commit 5 months ago
b72zhou added 1 commit 5 months ago
b72zhou added 1 commit 4 months ago
b72zhou added 1 commit 4 months ago
b72zhou added 1 commit 4 months ago
3be5395353 fix conflict
b72zhou added 1 commit 4 months ago
4c170aec2b resolve conflict
b72zhou added 1 commit 4 months ago
3d48211c10 fix setup
b72zhou added 1 commit 4 months ago
867ef9c84c support multiple data lines
b72zhou added 1 commit 4 months ago
991006ddb6 hover
b72zhou added 1 commit 4 months ago
472bf120b9 add tooltip
b72zhou added 1 commit 4 months ago
7f798d717d hover sometimes works
b72zhou added 1 commit 4 months ago
d1563d3380 fix ts error
a258wang reviewed 3 months ago
.lineText {
Owner

Seems like this class is unused

Seems like this class is unused
b72zhou marked this conversation as resolved
box-shadow: 0px calc(1rem / 16) calc(2rem / 16) var(--card-background);
pointer-events: none;
padding: calc(10rem / 16);
font-size: calc(18rem / 16);
Owner

NIT: maybe add a text-align: center to the tooltip?

NIT: maybe add a `text-align: center` to the tooltip?
b72zhou marked this conversation as resolved
interface LineGraphData {
x: string;
y: number;
Owner

Two nits here:

  1. How can we ensure that all lines use the same x-axis labels?
  2. Can we somehow encode the lineKey as part of the data prop passed into the component?

My idea would be to make the LineGraphData interface an object that holds more information, and then adjust the xAccessor and yAccessor functions to accomodate:

interface LineGraphData {
  xValues: string[];
  // could make this a separate type/interface, eg. LineData
  lines: {
    label: string;
    // yValues should be the same length as xValues
    yValues: number[];
  }[];
}

interface LineGraphProps {
  data: LineGraphData;
  // ...
}
Two nits here: 1. How can we ensure that all lines use the same x-axis labels? 2. Can we somehow encode the `lineKey` as part of the `data` prop passed into the component? My idea would be to make the `LineGraphData` interface an object that holds more information, and then adjust the `xAccessor` and `yAccessor` functions to accomodate: ```typescript interface LineGraphData { xValues: string[]; // could make this a separate type/interface, eg. LineData lines: { label: string; // yValues should be the same length as xValues yValues: number[]; }[]; } interface LineGraphProps { data: LineGraphData; // ... } ```
const DEFAULT_LABEL_SIZE = 16;
const lineKey = ["Java", "C++"];
Owner

This probably shouldn't be hard coded in the final component 😛

This probably shouldn't be hard coded in the final component 😛
xLabelSize = DEFAULT_LABEL_SIZE,
yLabelSize = DEFAULT_LABEL_SIZE,
}: LineGraphProps) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call
Owner

Is it possible to cast this to a type (maybe visx has a type we can import?) to get rid of the unsafe assignment complaints?

Is it possible to cast this to a type (maybe visx has a type we can import?) to get rid of the unsafe assignment complaints?
const customTheme = buildChartTheme({
// colors
backgroundColor: Color.secondaryBackground, // used by Tooltip, Annotation
colors: [Color.primaryAccent, Color.primaryHeading], // categorical colors, mapped to series via `dataKey`s
Owner

Do we have a Color.secondaryAccent which can be used instead of Color.primaryHeading?

Do we have a `Color.secondaryAccent` which can be used instead of `Color.primaryHeading`?
b72zhou marked this conversation as resolved
const yAccessor = (d: LineGraphData) => d.y;
return (
<>
Owner

Visx components should be wrapped in <svg>.

Visx components should be wrapped in `<svg>`.
Poster
Collaborator

Yup I was trying to wrap it in <svg>, but each time I did that the component was cut and only the left half of it is shown. I checked the document and examples of xychart component and all examples omitted svg. I may leave it like this right now but I may keep figuring out what happened behind it.

Yup I was trying to wrap it in `<svg>`, but each time I did that the component was cut and only the left half of it is shown. I checked the document and examples of xychart component and all examples omitted `svg`. I may leave it like this right now but I may keep figuring out what happened behind it.
y={0}
width={width}
height={height}
fill={Color.primaryBackground}
Owner

Is this rect necessary? I wonder if not specifying a background means that the svg will just have a transparent background, which would be sufficient since the page is already the right colour.

Is this `rect` necessary? I wonder if not specifying a background means that the svg will just have a transparent background, which would be sufficient since the page is already the right colour.
b72zhou marked this conversation as resolved
<div>{tooltipData?.nearestDatum?.key}</div>
<br />
{/* {xAccessor(tooltipData.nearestDatum.datum)}
{": "} */}
Owner

We should probably get rid of this commented out code 😛

We should probably get rid of this commented out code 😛
b72zhou marked this conversation as resolved
b72zhou added 1 commit 3 months ago
e6c77c750f fix from pr
b72zhou added 1 commit 3 months ago
21e146d66d last edit
b72zhou closed this pull request 2 months ago
All checks were successful
continuous-integration/drone/push Build is passing
Required
Details
Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/cs-2022-class-profile#21
Loading…
There is no content yet.