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

Bar Graph Component #16

Merged
a258wang merged 16 commits from a258wang-bar-graph into main 8 months ago
Conversation 1 Commits 16 Files Changed 8
a258wang commented 10 months ago
Owner

Adds a <BarGraphHorizontal /> component and a <BarGraphVertical /> component.

Closes #1.

Possible changes for the future:

  • Refactor to make the horizontal and vertical bar graphs into one component
  • Add (optional) graph title

https://a258wang-bar-graph-csc-class-profile-staging-snedadah.k8s.csclub.cloud/playground/

Adds a `<BarGraphHorizontal />` component and a `<BarGraphVertical />` component. Closes #1. Possible changes for the future: - Refactor to make the horizontal and vertical bar graphs into one component - Add (optional) graph title https://a258wang-bar-graph-csc-class-profile-staging-snedadah.k8s.csclub.cloud/playground/
a258wang added 8 commits 10 months ago
a258wang force-pushed a258wang-bar-graph from 5709c76a81 to da3c9f92bb 9 months ago
a258wang force-pushed a258wang-bar-graph from da3c9f92bb to 293183064a 9 months ago
a258wang added 1 commit 9 months ago
continuous-integration/drone/push Build is passing
Details
77d0a8a93d Add optional axis labels
a258wang changed title from WIP: Bar Graph Component to Bar Graph Component 9 months ago
a258wang requested review from j285he 9 months ago
a258wang requested review from snedadah 9 months ago
a258wang requested review from b72zhou 9 months ago
a258wang requested review from e26chiu 9 months ago
snedadah approved these changes 8 months ago
snedadah left a comment
Owner

Looks good to me! And yeah we definitely should add the title (though maybe we should instead have some sort of wrapper title component for all the graphs)

Looks good to me! And yeah we definitely should add the title (though maybe we should instead have some sort of wrapper title component for all the graphs)
components/BarGraph.module.css Outdated
.barText {
visibility: hidden;
font-family: "Inconsolata";
snedadah commented 8 months ago
Owner

Do we need to specify the font family on all of these even though the font is set globally for the page?
Also small nit: If we do need to set the font, should we add the monospace fallback here?

Do we need to specify the font family on all of these even though the font is set globally for the page? Also small nit: If we do need to set the font, should we add the monospace fallback here?
a258wang commented 8 months ago
Poster
Owner

I just tried it, and it looks like the visx Text component automatically uses Arial if you don't specify a font, so we either have to pass the font family as a prop to the Text component, or we have to specify the font on each class as done here.

Good catch on adding the monospace fallback though.

I just tried it, and it looks like the visx Text component automatically uses Arial if you don't specify a font, so we either have to pass the font family as a prop to the Text component, or we have to specify the font on each class as done here. Good catch on adding the monospace fallback though.
a258wang added 1 commit 8 months ago
continuous-integration/drone/push Build is passing
Details
21188553a8 Add monospace fallback
a258wang commented 8 months ago
Poster
Owner

I'm going to merge this without the graph title for now, and we can always come back later to add it if we want - in the meantime, throwing a heading above the graph would work in a pinch lol

I'm going to merge this without the graph title for now, and we can always come back later to add it if we want - in the meantime, throwing a heading above the graph would work in a pinch lol
a258wang merged commit 6735c52914 into main 8 months ago
a258wang deleted branch a258wang-bar-graph 8 months ago
a258wang referenced this issue from a commit 8 months ago
Bar Graph Component (#16)

Reviewers

j285he was requested for review 9 months ago
b72zhou was requested for review 9 months ago
e26chiu was requested for review 9 months ago
snedadah approved these changes 8 months ago
continuous-integration/drone/push Build is passing
Details
The pull request has been merged as 6735c52914.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
j285he
b72zhou
e26chiu
snedadah
Labels
Apply labels
Clear labels
Bug Component Config Good First Issue Low-Priority Page
No Label Bug Component Config Good First Issue Low-Priority Page
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/cs-2022-class-profile#16
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'a258wang-bar-graph'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes