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

Change Graphs to use tooltips (Closes #97) #112

Merged
e26chiu merged 3 commits from change-to-tooltips into main 2022-12-21 16:23:33 -05:00
Conversation 0 Commits 3 Files Changed 5 +867 -781
e26chiu commented 2022-12-16 16:37:00 -05:00
Contributor
Copy Link

Changes:

  • Remove Text that is displayed when hovering over the bar in the Bar Graph and Grouped Bar Graph - DIDN'T DO IT FOR Pie Charts since I wasn't sure if we still want the text or no.
  • Add tooltips for Bar Graph + Grouped Bar Graph + Pie Chart

NOTE: weird tooltip position with PieChart, might be related to other tooltip bugs we have

Used this approach for the PieChart: https://codesandbox.io/s/mjp8lmvop?expanddevtools=1
The only approach that worked with the PieChart required to have the PieSlice code within the PieChart component.

Improvement we can make:

  • For the Pie Chart, we could display the percentage and the number of answers in the tooltip to provide more context.
  • For the Pie Chart, we could also display label in tooltip.
Changes: - Remove Text that is displayed when hovering over the bar in the Bar Graph and Grouped Bar Graph - DIDN'T DO IT FOR Pie Charts since I wasn't sure if we still want the text or no. - Add tooltips for Bar Graph + Grouped Bar Graph + Pie Chart NOTE: weird tooltip position with PieChart, might be related to other tooltip bugs we have Used this approach for the PieChart: https://codesandbox.io/s/mjp8lmvop?expanddevtools=1 The only approach that worked with the PieChart required to have the PieSlice code within the PieChart component. Improvement we can make: - For the Pie Chart, we could display the percentage and the number of answers in the tooltip to provide more context. - For the Pie Chart, we could also display label in tooltip.
e26chiu added 3 commits 2022-12-16 16:37:01 -05:00
continuous-integration/drone/push Build is passing Details
50a637755b Change Grouped Bar Graph to use tooltips
continuous-integration/drone/push Build is passing Details
074828d7ae Change Pie Char to use tooltips
snedadah approved these changes 2022-12-21 12:07:19 -05:00
snedadah left a comment
Owner
Copy Link

Nice work!! I think we should also remove the text for the pie chart too, the tooltip is much better!

I totally with thoese improvments, I think we should try to make our pie charts similar to the syde pie charts since those are very good!

In terms of the centering issue, it is known, and I will fix it when I get to #89.

Nice work!! I think we should also remove the text for the pie chart too, the tooltip is much better! I totally with thoese improvments, I think we should try to make our pie charts similar to the syde pie charts since those are very good! In terms of the centering issue, it is known, and I will fix it when I get to https://git.csclub.uwaterloo.ca/www/cs-2022-class-profile/issues/89.
👍 1 ❤️ 1
e26chiu merged commit 550029feb0 into main 2022-12-21 16:23:33 -05:00
e26chiu deleted branch change-to-tooltips 2022-12-21 16:23:34 -05:00
Sign in to join this conversation.
Reviewers
No reviewers
snedadah
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#112
No description provided.
Delete Branch "change-to-tooltips"

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?