Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Horizontal bar for time overview #590

Merged
merged 17 commits into from
Aug 29, 2022
Merged

Horizontal bar for time overview #590

merged 17 commits into from
Aug 29, 2022

Conversation

HenrikeW
Copy link
Contributor

@HenrikeW HenrikeW commented Aug 29, 2022

Related issue(s) and PR(s)

This PR relates to #580.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

List of changes made

  • a new component for a stacked horizontal bar was added
  • the bar is displayed on the report page
  • it fetches its own data about time entries from the past 12 months

Screenshot of the fix

image

Testing

  • go to the report page and scroll down to see your personal overview bar

Further comments

  • the BarChart component fetches data in the same way that the WiP SpentTime page / component does. If we decide on having both the horizontal bar and a separate page, this should be refactored so that data is fetched centrally at one place.

Definition of Done checklist

  • I have made an effort making the commit history understandable
  • I have performed a self-review of my own code and commented any hard-to-understand areas
  • Tests and lint/format validations are passing
  • My changes generate no new warnings

@HenrikeW HenrikeW force-pushed the dev/horizontal-bar branch from 6890930 to ca7af4a Compare August 29, 2022 07:57
@HenrikeW HenrikeW changed the title Horizantal bar for time overview Horizontal bar for time overview Aug 29, 2022
@HenrikeW HenrikeW marked this pull request as ready for review August 29, 2022 08:11
@HenrikeW HenrikeW requested review from jonandernovella and a user August 29, 2022 08:11
@kusalananda
Copy link
Member

kusalananda commented Aug 29, 2022

Are the elements of the bar sorted in any particular way? Is it guaranteed always to show the activities in the same order?

EDIT: No, it appears to sort the activities in most-recently-used order (or something similar, possibly semi-randomly).

EDIT: The colors are assigned left-to-right, not to the specific activities.

Suggestions:

  1. Sort activities by some criteria (most hours, alphabetically, or something else, or use a static ordering).
  2. Assign colours for the activities, and don't change these. Using static ordering of the activities in the bar would possibly solve this right now.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks nice!
Found a border-test issue. If no time has been reported, the bar is not visible. If time is then added and the user saves the changes, the bar does not appear until the page is reloaded.
Same thing when the bar is visible and time entries are saved, it does not alter until the page is reloaded.
Another issue is that the order of the activities seems to be random. Usage would be easier if they always appear in the same order and with the same color.
EDIT: Sorting makes it a lot better. Agree that colors should be fixed for activities.

@HenrikeW
Copy link
Contributor Author

HenrikeW commented Aug 29, 2022

Thanks for your feedback!
Now, the activities should be sorted alphabetically, there should be a short message when users don't have any time logged yet and the bar should update as soon as changes are saved.

I totally agree that the colors should be fixed for activities but that requires a full list of all possible activities. There might also be requirements on what type of activities should have what type of colors, so I'd like to create a new issue for that and address it in a later PR.

@HenrikeW HenrikeW requested review from a user and kusalananda August 29, 2022 09:20
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Ok. Sorting and updating after saving changes makes this good enough for now. We can look at deciding color-activity match later on.

@HenrikeW HenrikeW merged commit b2194ac into develop Aug 29, 2022
@HenrikeW HenrikeW deleted the dev/horizontal-bar branch August 29, 2022 10:47
@jonandernovella jonandernovella mentioned this pull request Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants