Add Demographics page (Closes #53) #73

Merged
e26chiu merged 8 commits from demographics-page into main 4 months ago
Collaborator

Some changes may need to be done in ComponentWrapper component to customize space + allowing ComponentWrapper without a bodyText prop.

I tried to use WordCloud on D7 and it is associating the wrong text to the wrong value.

I changed the colour of the text (might be changed later on) because pure white strains the eye (contrast is too high).

https://demographics-page-csc-class-profile-staging-snedadah.k8s.csclub.cloud/demographics/

Some changes may need to be done in `ComponentWrapper` component to customize space + allowing `ComponentWrapper` without a `bodyText` prop. I tried to use WordCloud on D7 and it is associating the wrong text to the wrong value. I changed the colour of the text (might be changed later on) because pure white strains the eye (contrast is too high). https://demographics-page-csc-class-profile-staging-snedadah.k8s.csclub.cloud/demographics/
e26chiu added 1 commit 5 months ago
915d792123 Add initial demographics page
e26chiu added 6 commits 5 months ago
e26chiu changed title from [WIP] Add Demographics page (Closes #53) to Add Demographics page (Closes #53) 5 months ago
e26chiu reviewed 5 months ago
const { width } = useWindowDimensions();
const isMobile = useIsMobile();
const defaultGraphWidth = isMobile ? width / 1.25 : width / 2;
Poster
Collaborator

I believe these values (defaultGraphWidth, defaultPieChartWidth, defaultGraphHeight, etc.) should be shared across all pages. We should probably create a util file like Color.ts to contain these values. It can be changed in another PR.

I believe these values (defaultGraphWidth, defaultPieChartWidth, defaultGraphHeight, etc.) should be shared across all pages. We should probably create a `util` file like `Color.ts` to contain these values. It can be changed in another PR.
e26chiu reviewed 5 months ago
align="right"
>
<BarGraphVertical
data={D9} // we might want a histogram instead
Poster
Collaborator

Will be changed when histogram component is merged.

Will be changed when histogram component is merged.
Owner

I reccomend adding
// TODO: change when histogram component is ready

I reccomend adding `// TODO: change when histogram component is ready`
e26chiu marked this conversation as resolved
snedadah approved these changes 4 months ago
snedadah left a comment
Owner

Nice work! LGTM with a small comment.

We should make another pr for doing the refactoring the common page styles. I'm also working on debugging the word cloud issue.

Nice work! LGTM with a small comment. We should make another pr for doing the refactoring the common page styles. I'm also working on debugging the word cloud issue.
data={D11}
width={defaultGraphWidth}
height={defaultGraphHeight}
margin={{
Owner

You can do
margin={ { ...defaultBarGraphMargin, ...{ left: 200 } } }
To use defaults with only one value changed. I also reccomend you use 250 instead of 200, since the "no" is still hidden on my screen with 200.

Also, this graph is pretty messed up on mobile, but I know that the other label pr will hopefully fix it.

You can do ` margin={ { ...defaultBarGraphMargin, ...{ left: 200 } } } ` To use defaults with only one value changed. I also reccomend you use 250 instead of 200, since the "no" is still hidden on my screen with 200. Also, this graph is pretty messed up on mobile, but I know that the other label pr will hopefully fix it.
e26chiu marked this conversation as resolved
e26chiu added 1 commit 4 months ago
f49a664eb9 Incorporate comment changes
e26chiu merged commit fb9c5243ab into main 4 months ago
e26chiu deleted branch demographics-page 4 months ago

Reviewers

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