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

refactor: year chart #69

Merged
merged 16 commits into from
Mar 1, 2024
Merged

refactor: year chart #69

merged 16 commits into from
Mar 1, 2024

Conversation

eldu
Copy link
Contributor

@eldu eldu commented Feb 26, 2024

  • refactor: YearChart.js and yearChart.json
    • Now when a year is clicked, it's just highlighted

TODOs that came from this PR

Demo

Untitled.mov

Old
Screenshot 2024-02-27 at 3 50 49 PM
Screenshot 2024-02-27 at 3 50 44 PM

@eldu eldu changed the base branch from main to build-update February 26, 2024 22:17
Copy link

github-actions bot commented Feb 26, 2024

Visit the preview URL for this PR (updated for commit 18249c1):

https://ccv-pubs--pr69-refactor-year-chart-r2hr8d1z.web.app

(expires Fri, 08 Mar 2024 21:16:30 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f75c16b3bdd7d81b7a0b7f8d3eed826b541e7a2a

@eldu eldu marked this pull request as ready for review February 27, 2024 21:08
@eldu eldu self-assigned this Feb 27, 2024
@eldu eldu requested a review from paulstey February 27, 2024 21:38
Copy link

@hetd54 hetd54 left a comment

Choose a reason for hiding this comment

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

This looks so cool! You've done a beautiful job.

I think the month view shows the same data no matter which month you are on currently. 2024 shows 3 pubs:

Screenshot 2024-02-27 at 5 28 35 PM

But the Month view for 2024 is showing:
Screenshot 2024-02-27 at 5 29 02 PM

It was also unclear to me as a user how to return to the year view. Should we add something visually instead of clicking on the table again?

@eldu
Copy link
Contributor Author

eldu commented Feb 28, 2024

@hetd54 Amazing catch! Somehow through my refactor I messed up the filter. Here's what 2024 looks like now :)

Screenshot 2024-02-28 at 11 38 03 AM

It was also unclear to me as a user how to return to the year view. Should we add something visually instead of clicking on the table again?

So, this is what the previous functionality was. I'm interested into hearing ideas though !

@hetd54
Copy link

hetd54 commented Feb 28, 2024

@hetd54 Amazing catch! Somehow through my refactor I messed up the filter. Here's what 2024 looks like now :)

Screenshot 2024-02-28 at 11 38 03 AM > It was also unclear to me as a user how to return to the year view. Should we add something visually instead of clicking on the table again?

So, this is what the previous functionality was. I'm interested into hearing ideas though !

Data looks as expected now!

For an idea, maybe a " ← Back to Year View" link somewhere beneath the title, or even below the graph? That way you can reserve the action of clicking on the bar for the table filter feature later.

@eldu
Copy link
Contributor Author

eldu commented Feb 28, 2024

  • Remove month view
  • Highlight selectedYear

@eldu eldu requested a review from hetd54 February 28, 2024 19:59
@hetd54
Copy link

hetd54 commented Feb 29, 2024

@eldu I would expect that clicking on a bar to highlight it instantly, but if a bar is already highlighted it unhighlights onclick no matter where on the graph you've clicked:

Screen.Recording.2024-02-29.at.10.57.54.AM.mov

@eldu eldu merged commit a2a73a6 into build-update Mar 1, 2024
2 checks passed
@eldu eldu deleted the refactor-year-chart branch March 1, 2024 21:17
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.

3 participants