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

Add Stacked Bar Graph component (Closes #3) #37

Merged
e26chiu merged 14 commits from stackedbar-component into main 2022-10-01 20:54:06 -04:00
Conversation 2 Commits 14 Files Changed 6 +608 -1
e26chiu commented 2022-08-29 14:42:42 -04:00
Contributor
Copy Link

Notes:

  • Add Stacked Bar Graph component using the BarStack component from Visx.
  • Hovering over a bar will add a highlight on the hovered bar and not the entire stacked bar (I have not yet found a way to highlight the entire stacked bar)
  • Hovering over a bar will also show a tooltip box with the data for each bar. (The number represents the height of the bar and not the cumulative stacked bar height)
  • In the mock data, the number given to each key represent their individual height (and not their overall value). The graph calculates the total height of the stacked bars.
  • Missing percentage next to the left axis labels
  • Resizable width, height, and margins. Customizable number of ticks for the y axis.

image

Staging: https://stackedbar-component-csc-class-profile-staging-snedadah.k8s.csclub.cloud/playground/

Notes: - Add Stacked Bar Graph component using the `BarStack` component from Visx. - Hovering over a bar will add a highlight on the hovered bar and not the entire stacked bar (I have not yet found a way to highlight the entire stacked bar) - Hovering over a bar will also show a tooltip box with the data for each bar. (The number represents the height of the bar and not the cumulative stacked bar height) - In the mock data, the number given to each key represent their individual height (and not their overall value). The graph calculates the total height of the stacked bars. - Missing percentage next to the left axis labels - Resizable width, height, and margins. Customizable number of ticks for the y axis. ![image](/attachments/0ef5c998-c6c3-4e36-971e-6a33fe72ed1e) Staging: https://stackedbar-component-csc-class-profile-staging-snedadah.k8s.csclub.cloud/playground/
image.png
36 KiB
a258wang was assigned by e26chiu 2022-08-29 14:42:42 -04:00
e26chiu added 6 commits 2022-08-29 14:42:43 -04:00
continuous-integration/drone/push Build is failing Details
2496e34798 add boxplot
continuous-integration/drone/push Build is failing Details
c3b33493e5 add stacked bar graph component
continuous-integration/drone/push Build is failing Details
4cd27dbbaf move legend to the right of the graph
e26chiu added 1 commit 2022-08-29 14:44:16 -04:00
continuous-integration/drone/push Build is passing Details
35463a6718 delete CandleStick graph
a258wang removed their assignment 2022-08-29 21:12:38 -04:00
a258wang reviewed 2022-08-30 16:02:38 -04:00
a258wang left a comment
Owner
Copy Link

A few visual nitpicks:

  1. The x-axis names aren't centered with the bars, and I think the bars aren't horizontally centered in their columns either.
    image
  2. The y-axis values aren't centered with the lines.
    image
  3. Horizontal lines stick out on the right side.
    image
A few visual nitpicks: 1. The x-axis names aren't centered with the bars, and I think the bars aren't horizontally centered in their columns either. ![image](/attachments/3fe16c99-3793-4109-bb63-dc88742ceda3) 2. The y-axis values aren't centered with the lines. ![image](/attachments/f225ef07-4cd6-4f16-9123-446f98c979ea) 3. Horizontal lines stick out on the right side. ![image](/attachments/5c50d12e-5e80-4fb8-a749-6b3e55e986b9)
image.png
15 KiB
image.png
2.0 KiB
image.png
2.0 KiB
image.png
2.8 KiB
image.png
components/StackedBarGraph.module.css Outdated
@ -0,0 +9,4 @@
.legend {
position: absolute;
display: flex;
font-size: 16px;
a258wang commented 2022-08-30 15:54:56 -04:00
Owner
Copy Link

NIT: calc(16rem / 16) instead of px in this file

NIT: `calc(16rem / 16)` instead of `px` in this file
components/StackedBarGraph.tsx
@ -0,0 +15,4 @@
interface StackedBarData {
category: string;
[key: string]: number | string;
a258wang commented 2022-08-30 15:53:49 -04:00
Owner
Copy Link

Have we considered

interface StackedBarData {
  category: string;
  values: {
    [key: string]: number;
  };
}

?
This would allow for a key called "category", at the expense of more nesting.

Have we considered ```typescript interface StackedBarData { category: string; values: { [key: string]: number; }; } ``` ? This would allow for a key called "category", at the expense of more nesting.
a258wang commented 2022-08-30 15:54:16 -04:00
Owner
Copy Link

Also is there a reason why the value can be number | string and not just number? EDIT: nvm, it's to make TS stop complaining

https://stackoverflow.com/questions/54460029/typescript-inteface-where-id-must-be-number-and-other-properties-must-be-strin

~~Also is there a reason why the value can be `number | string` and not just `number`?~~ EDIT: nvm, it's to make TS stop complaining https://stackoverflow.com/questions/54460029/typescript-inteface-where-id-must-be-number-and-other-properties-must-be-strin
e26chiu commented 2022-09-03 08:50:52 -04:00
Author
Contributor
Copy Link

I have considered the above, but it doesn't work since the format of the data doesn't match what is expected in the props data taken by the <BarStack> component:

<BarStack<StackedBarData, string> 
   data={data}
   keys={keys}

It specifically needs category to be a key in data and all the other keys to be directly nested inside of data (cannot be nested inside of values).

I have considered the above, but it doesn't work since the format of the data doesn't match what is expected in the props `data` taken by the `<BarStack>` component: ``` <BarStack<StackedBarData, string> data={data} keys={keys} ``` It specifically needs `category` to be a key in `data` and all the other keys to be directly nested inside of `data` (cannot be nested inside of `values`).
components/StackedBarGraph.tsx Outdated
@ -0,0 +62,4 @@
const tooltipStyles = {
...defaultStyles,
minWidth: 60,
backgroundColor: Color.primaryAccentLighter,
a258wang commented 2022-08-30 15:55:31 -04:00
Owner
Copy Link

NIT: background should be Color.label

NIT: background should be `Color.label`
components/StackedBarGraph.tsx Outdated
@ -0,0 +120,4 @@
showTooltip,
} = useTooltip<TooltipData>();
const { containerRef, TooltipInPortal } = useTooltipInPortal({
a258wang commented 2022-08-30 15:56:09 -04:00
Owner
Copy Link

Any reason why we're using the TooltipInPortal here instead of the regular Tooltip we're using everywhere else?

Any reason why we're using the TooltipInPortal here instead of the regular Tooltip we're using everywhere else?
components/StackedBarGraph.tsx Outdated
@ -0,0 +126,4 @@
scroll: true,
});
if (width < 10) return null;
a258wang commented 2022-08-30 15:56:49 -04:00
Owner
Copy Link

NIT: redundant if we have return width < 10 ? null : ... down below

NIT: redundant if we have `return width < 10 ? null : ...` down below
components/StackedBarGraph.tsx Outdated
@ -0,0 +129,4 @@
if (width < 10) return null;
// bounds
const xMax = width;
const yMax = height - margin.top - 50;
a258wang commented 2022-08-30 15:57:22 -04:00
Owner
Copy Link

Just curious, why 50?

Just curious, why 50?
components/StackedBarGraph.tsx Outdated
@ -0,0 +211,4 @@
<AxisBottom
top={yMax}
scale={xScale}
left={valueAxisLeftOffset / 5}
a258wang commented 2022-08-30 15:57:39 -04:00
Owner
Copy Link

Why divide by 5?

Why divide by 5?
a258wang commented 2022-08-30 17:13:58 -04:00
Owner
Copy Link

Also is there the possibility of being able to support a horizontal stacked bar graph as well?

Also is there the possibility of being able to support a horizontal stacked bar graph as well?
e26chiu added 1 commit 2022-09-02 16:58:46 -04:00
continuous-integration/drone/push Build is passing Details
feb7191a2d remove boxplot
e26chiu added 5 commits 2022-09-03 14:07:49 -04:00
e26chiu commented 2022-09-03 14:10:32 -04:00
Author
Contributor
Copy Link

Horizontal Stacked Bar Graph looks like this:

image

Note: I was unable to center the labels of the value axis (x-axis)

Horizontal Stacked Bar Graph looks like this: ![image](/attachments/3e52ac23-a797-4a9a-925d-e1e1d868ca78) Note: I was unable to center the labels of the value axis (x-axis)
image.png
28 KiB
a258wang approved these changes 2022-09-08 02:29:36 -04:00
a258wang left a comment
Owner
Copy Link

Just left one last comment.

I'm going to approve this so we can merge it for now, but I'd like for us to come back later to fix some visual nitpicks (eg. axis lines sticking out, axis tick label centering - could possibly check out the BarGraph to see how the alignment is done) and maybe clean up the code since there is quite a bit of repetition between the vertical and horizontal graphs.

EDIT: #51

Just left one last comment. I'm going to approve this so we can merge it for now, but I'd like for us to come back later to fix some visual nitpicks (eg. axis lines sticking out, axis tick label centering - could possibly check out the BarGraph to see how the alignment is done) and maybe clean up the code since there is quite a bit of repetition between the vertical and horizontal graphs. EDIT: #51
components/StackedBarGraph.tsx
@ -0,0 +324,4 @@
const xMax = width;
const yMax = height - margin.top - axisBottomOffset;
categoryScale.rangeRound([yMax, 0]);
a258wang commented 2022-09-08 02:25:50 -04:00
Owner
Copy Link

Please change the range to [0, yMax] - it makes it so the order of the categories is the same top-to-bottom when the graph has horizontal bars vs. left-to-right when the graph has vertical bars.

Please change the range to `[0, yMax]` - it makes it so the order of the categories is the same top-to-bottom when the graph has horizontal bars vs. left-to-right when the graph has vertical bars.
e26chiu added 1 commit 2022-10-01 20:49:58 -04:00
e26chiu merged commit 2d34b84cb0 into main 2022-10-01 20:54:06 -04:00
e26chiu deleted branch stackedbar-component 2022-10-01 20:54:06 -04:00
Sign in to join this conversation.
Reviewers
No reviewers
a258wang
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#37
No description provided.
Delete Branch "stackedbar-component"

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?