www
/
cs-2022-class-profile
16
0
Fork 0
Code Issues 11 Pull Requests 1 Projects Releases Wiki Activity

WIP: Line Graph Component #21

Closed
b72zhou wants to merge 14 commits from b72zhou-line-graph into main
pull from: b72zhou-line-graph
merge into: www:main
Conversation 0 Commits 14 Files Changed 7 +685 -37
b72zhou commented 2022-06-22 15:37:10 -04:00
Contributor
Copy Link

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 2022-06-22 15:37:11 -04:00
b72zhou added 1 commit 2022-06-22 15:37:22 -04:00
b72zhou added 1 commit 2022-08-10 20:12:35 -04:00
b72zhou added 1 commit 2022-08-10 21:00:45 -04:00
b72zhou added 1 commit 2022-08-17 10:37:11 -04:00
continuous-integration/drone/push Build is failing Details
3be5395353 fix conflict
b72zhou added 1 commit 2022-08-17 10:43:20 -04:00
continuous-integration/drone/push Build is failing Details
4c170aec2b resolve conflict
b72zhou added 1 commit 2022-08-17 11:02:47 -04:00
continuous-integration/drone/push Build is passing Details
3d48211c10 fix setup
b72zhou added 1 commit 2022-08-17 12:28:47 -04:00
continuous-integration/drone/push Build is passing Details
867ef9c84c support multiple data lines
b72zhou added 1 commit 2022-08-17 18:38:47 -04:00
continuous-integration/drone/push Build is failing Details
991006ddb6 hover
b72zhou added 1 commit 2022-08-18 11:10:15 -04:00
continuous-integration/drone/push Build is failing Details
472bf120b9 add tooltip
b72zhou added 1 commit 2022-08-18 11:32:46 -04:00
continuous-integration/drone/push Build is failing Details
7f798d717d hover sometimes works
b72zhou added 1 commit 2022-08-18 11:52:29 -04:00
continuous-integration/drone/push Build is passing Details
d1563d3380 fix ts error
a258wang reviewed 2022-08-24 20:58:33 -04:00
components/LineGraph.module.css Outdated
@ -0,0 +1,35 @@
.lineText {
a258wang commented 2022-08-24 20:34:23 -04:00
Owner
Copy Link

Seems like this class is unused

Seems like this class is unused
b72zhou marked this conversation as resolved
components/LineGraph.module.css Outdated
@ -0,0 +30,4 @@
box-shadow: 0px calc(1rem / 16) calc(2rem / 16) var(--card-background);
pointer-events: none;
padding: calc(10rem / 16);
font-size: calc(18rem / 16);
a258wang commented 2022-08-24 20:35:04 -04:00
Owner
Copy Link

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
components/LineGraph.tsx
@ -0,0 +13,4 @@
interface LineGraphData {
x: string;
y: number;
a258wang commented 2022-08-24 20:51:31 -04:00
Owner
Copy Link

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; // ... } ```
components/LineGraph.tsx
@ -0,0 +32,4 @@
const DEFAULT_LABEL_SIZE = 16;
const lineKey = ["Java", "C++"];
a258wang commented 2022-08-24 20:35:41 -04:00
Owner
Copy Link

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

This probably shouldn't be hard coded in the final component 😛
components/LineGraph.tsx
@ -0,0 +42,4 @@
xLabelSize = DEFAULT_LABEL_SIZE,
yLabelSize = DEFAULT_LABEL_SIZE,
}: LineGraphProps) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call
a258wang commented 2022-08-24 20:55:10 -04:00
Owner
Copy Link

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?
components/LineGraph.tsx Outdated
@ -0,0 +46,4 @@
const customTheme = buildChartTheme({
// colors
backgroundColor: Color.secondaryBackground, // used by Tooltip, Annotation
colors: [Color.primaryAccent, Color.primaryHeading], // categorical colors, mapped to series via `dataKey`s
a258wang commented 2022-08-24 20:52:48 -04:00
Owner
Copy Link

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
components/LineGraph.tsx Outdated
@ -0,0 +61,4 @@
const yAccessor = (d: LineGraphData) => d.y;
return (
<>
a258wang commented 2022-08-24 20:56:02 -04:00
Owner
Copy Link

Visx components should be wrapped in <svg>.

Visx components should be wrapped in `<svg>`.
b72zhou commented 2022-08-31 15:18:46 -04:00
Author
Contributor
Copy Link

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.
components/LineGraph.tsx Outdated
@ -0,0 +67,4 @@
y={0}
width={width}
height={height}
fill={Color.primaryBackground}
a258wang commented 2022-08-24 20:57:37 -04:00
Owner
Copy Link

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
components/LineGraph.tsx Outdated
@ -0,0 +129,4 @@
<div>{tooltipData?.nearestDatum?.key}</div>
<br />
{/* {xAccessor(tooltipData.nearestDatum.datum)}
{": "} */}
a258wang commented 2022-08-24 20:58:21 -04:00
Owner
Copy Link

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 2022-08-31 18:58:46 -04:00
continuous-integration/drone/push Build is passing Details
e6c77c750f fix from pr
b72zhou added 1 commit 2022-08-31 20:00:21 -04:00
continuous-integration/drone/push Build is passing Details
21e146d66d last edit
b72zhou closed this pull request 2022-09-20 16:16:18 -04:00
All checks were successful
continuous-integration/drone/push Build is passing
Required
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
Bug

   
Component

   
Config

   
Good First Issue

   
Low-Priority

   
Page
No Label
Bug
Component
Config
Good First Issue
Low-Priority
Page
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/cs-2022-class-profile#21
No description provided.
Delete Branch "b72zhou-line-graph"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?