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

Merged
e26chiu merged 14 commits from stackedbar-component into main 2 months ago
Collaborator

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/
a258wang was assigned by e26chiu 3 months ago
e26chiu added 6 commits 3 months ago
e26chiu added 1 commit 3 months ago
35463a6718 delete CandleStick graph
a258wang removed their assignment 3 months ago
a258wang reviewed 3 months ago
a258wang left a comment
Owner

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)
.legend {
position: absolute;
display: flex;
font-size: 16px;
Owner

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

NIT: `calc(16rem / 16)` instead of `px` in this file
interface StackedBarData {
category: string;
[key: string]: number | string;
Owner

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.
Owner

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
Poster
Collaborator

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`).
const tooltipStyles = {
...defaultStyles,
minWidth: 60,
backgroundColor: Color.primaryAccentLighter,
Owner

NIT: background should be Color.label

NIT: background should be `Color.label`
showTooltip,
} = useTooltip<TooltipData>();
const { containerRef, TooltipInPortal } = useTooltipInPortal({
Owner

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?
scroll: true,
});
if (width < 10) return null;
Owner

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

NIT: redundant if we have `return width < 10 ? null : ...` down below
if (width < 10) return null;
// bounds
const xMax = width;
const yMax = height - margin.top - 50;
Owner

Just curious, why 50?

Just curious, why 50?
<AxisBottom
top={yMax}
scale={xScale}
left={valueAxisLeftOffset / 5}
Owner

Why divide by 5?

Why divide by 5?
Owner

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 3 months ago
feb7191a2d remove boxplot
e26chiu added 5 commits 3 months ago
Poster
Collaborator

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)
a258wang approved these changes 3 months ago
a258wang left a comment
Owner

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
const xMax = width;
const yMax = height - margin.top - axisBottomOffset;
categoryScale.rangeRound([yMax, 0]);
Owner

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 2 months ago
e26chiu merged commit 2d34b84cb0 into main 2 months ago
e26chiu deleted branch stackedbar-component 2 months ago

Reviewers

a258wang approved these changes 3 months ago
continuous-integration/drone/push Build is passing
The pull request has been merged as 2d34b84cb0.
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#37
Loading…
There is no content yet.