Skip to content
This repository has been archived by the owner on Jul 27, 2024. It is now read-only.

[FE] Analytics - Progress bars & co. #117

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

[FE] Analytics - Progress bars & co. #117

wants to merge 8 commits into from

Conversation

rbenjos
Copy link
Collaborator

@rbenjos rbenjos commented Jun 29, 2021

Added an additional view for analytics with some dummy charts,
and a functional progress bar.

@rbenjos rbenjos requested review from ScDor and noabarlia June 29, 2021 16:05
Copy link
Owner

@ScDor ScDor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job, that's an important feature!
Please see the comments and comment so we can merge this soon

label: "total points",
backgroundColor: "#999999",
fill: false,
data: [0,25, 56, 70, 80, 90, 105, 125, 134],
Copy link
Owner

@ScDor ScDor Jun 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

134? What's this? (as well as all other hardcoded values)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the number of points required for completing a CS degree.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Does the order mean anything?
  2. is it hardcoded for development purposes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. no, it approximates the cummulative number of points up to the appropriate semseter
  2. it is hardcoded

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. why?

mounted () {
// Overwriting base render method with actual data.
this.renderChart({
labels: ['Must', 'Choose from list', 'Choice'],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have CornerStone and Complementary as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, what are the numbers representing their types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i need this also for the progress bars, even though complementary courses would probably not appear there, as there is no quota of them that needs to be filled

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of numbers representing types on the backend, I guess someone created such a mapping somewhere in the FE

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Track model does have the quota, is it missing from the API?

Closure_Front_End/src/components/PieChart.vue Show resolved Hide resolved
datasets: [
{
backgroundColor: ['#bc87d0','#fbaf5d','#f06eaa'],
data: [70, 20, 40]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do these values mean?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just an approximate distribution of your points on a standard single major degree, it's just a dummy graph

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a dummy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because untill we find a way communicating the data between different views, i can't access them from the analytics view. thats an architectural decision that noa should do i believe

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Is this feature as part of this PR necessary for creating the infrastructure?
  2. Can we hide it until it's ready? (so master doesn't have half-baked features)

mounted () {
// Overwriting base render method with actual data.
this.renderChart({
labels: ['Must', 'Choose from list', 'Choice'],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we handle cases when one category (or more) require 0 points (so - isn't required)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this graph it really doesn't matter as they just won't appear, but in the progress bars i'll probably need to drop the cornerStones bar when a certain track is taken (like cognition, which doesn't require corner stones)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, please do, and notice various courses may even have 0 choice or 0 from_list (do I hear a 0 must?)

Closure_Front_End/src/components/ProgressBox.vue Outdated Show resolved Hide resolved
@ScDor
Copy link
Owner

ScDor commented Jun 29, 2021

Does it close any existing issues? if so, please mention them (fixes: #xx with xx being the relevant issue number)

@ScDor ScDor changed the title Analytics [FE] Analytics - Progress bars & co. Jun 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants