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

Expand / collapse all categories #1143

Merged

Conversation

joel-jeremy
Copy link
Contributor

@joel-jeremy joel-jeremy commented Jun 16, 2023

This PR is for the first item listed in #559: Expand All / Collapse All Categories

For the expand / collapse all categories functionality, I was choosing between having a single Expand / collapse all categories button or one for each: Expand all categories and Collapse all categories buttons.

For the initial implementation, I have opted with the latter. Please let me know which one is the right way to go or if there are other suggestions and I'll just accordingly.

image

@netlify
Copy link

netlify bot commented Jun 16, 2023

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 690dfdd
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/6490888e3b9a370007023b50
😎 Deploy Preview https://deploy-preview-1143--actualbudget.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@joel-jeremy joel-jeremy changed the title Expand and collapse all categories Expand / collapse all categories Jun 16, 2023
@joel-jeremy joel-jeremy force-pushed the 559-expand-collapse-all-categories branch 3 times, most recently from 8552397 to a351dee Compare June 16, 2023 21:24
@j-f1
Copy link
Contributor

j-f1 commented Jun 17, 2023

I’m torn on what I think the right behavior should be. My first thought is that it should match the expand/collapse split transactions button. But that button has very non-intuitive behavior (it only changes state when you click it, not when you manually toggle split transactions). We also have the luxury of space for two buttons here, if we want to. What do you think, @MatissJanis?

@MatissJanis
Copy link
Member

My framework for adding buttons is this:

Will the added feature be used by most users a lot of times per session?

Yes: it deserves prime real-estate.

No: lets hide it in a menu or other place as to not overwhelm the user with many unnecessary buttons

In this case it's a "no". So IMO hiding it in the small menu is fine. If we get a lot of people complaining that it's hard to find this - we can revisit the issue.

TLDR: I'm OK with the proposed idea from a design point of view.

@carkom
Copy link
Contributor

carkom commented Jun 17, 2023

Does or make sense to remove the word 'categories" from these 3 menu options?

Seeing as how you are clicking a menu button right next to the word category it's not a difficult jump to assume all the menu options are for the categories so no need to write it out 3 times. Just a thought...

@joel-jeremy joel-jeremy force-pushed the 559-expand-collapse-all-categories branch from a351dee to a5b5161 Compare June 19, 2023 02:34
@joel-jeremy
Copy link
Contributor Author

Removed the word categories in the menu options. Also thinking of renaming "Toggle hidden" to "Toggle visibility". Thoughts?

@joel-jeremy joel-jeremy force-pushed the 559-expand-collapse-all-categories branch from bfc1006 to 690dfdd Compare June 19, 2023 16:55
@tjfinlinson
Copy link

Removed the word categories in the menu options. Also thinking of renaming "Toggle hidden" to "Toggle visibility". Thoughts?

I think keeping it to Toggle hidden is the right wording to use. It's a closer match to the 'hide' option in the individual category dropdown menu.

Copy link
Member

@MatissJanis MatissJanis left a comment

Choose a reason for hiding this comment

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

Amazing! Thanks for your contribution @joel-jeremy !

(I'll leave the PR open for a few more days in case any other maintainers wants to review, but on my end this looks good to go)

Copy link
Contributor

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@j-f1 j-f1 merged commit 497a310 into actualbudget:master Jun 20, 2023
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved labels Jun 20, 2023
@j-f1
Copy link
Contributor

j-f1 commented Jun 20, 2023

Thank you for contributing!

TomAFrench added a commit to TomAFrench/actual that referenced this pull request Jun 24, 2023
* master: (54 commits)
  New linter rule import/no-unused-modules & fixing import on typescript (actualbudget#1173)
  React Router 6 fixes (actualbudget#1178)
  Remove remaining tutorial code (actualbudget#1174)
  react-router 6 upgrade (actualbudget#1066)
  Deleting all unused files, deleting unused functions from loot-core (actualbudget#1158)
  Log more details when migrations are out of sync (actualbudget#1161)
  Remove upgrade notifications code (actualbudget#1156)
  🔥  removing needs-triage github label (actualbudget#1157)
  Tidy up exports in loot-core (actualbudget#1147)
  🐛  remove 'we have been notified' copy (actualbudget#1155)
  Updates to the server button at the top right (actualbudget#1141)
  Expand / collapse all categories (actualbudget#1143)
  ✨ (nordigen) release the feature (actualbudget#1135)
  Improve error logging in the API (actualbudget#1121)
  ♻️ (crdt) moved re-used utils in actual-server to separate package (actualbudget#1150)
  Removing Tutorial code (actualbudget#1146)
  Removing unused functions (actualbudget#1145)
  Revert “Make number parsing agnostic to decimal and thousands separators” (actualbudget#1144)
  Strip a trailing slash off of server URLs (actualbudget#1140)
  Update CONTRIBUTING.md to point to the website (actualbudget#1138)
  ...
TomAFrench added a commit to TomAFrench/actual that referenced this pull request Jun 24, 2023
* master: (54 commits)
  New linter rule import/no-unused-modules & fixing import on typescript (actualbudget#1173)
  React Router 6 fixes (actualbudget#1178)
  Remove remaining tutorial code (actualbudget#1174)
  react-router 6 upgrade (actualbudget#1066)
  Deleting all unused files, deleting unused functions from loot-core (actualbudget#1158)
  Log more details when migrations are out of sync (actualbudget#1161)
  Remove upgrade notifications code (actualbudget#1156)
  🔥  removing needs-triage github label (actualbudget#1157)
  Tidy up exports in loot-core (actualbudget#1147)
  🐛  remove 'we have been notified' copy (actualbudget#1155)
  Updates to the server button at the top right (actualbudget#1141)
  Expand / collapse all categories (actualbudget#1143)
  ✨ (nordigen) release the feature (actualbudget#1135)
  Improve error logging in the API (actualbudget#1121)
  ♻️ (crdt) moved re-used utils in actual-server to separate package (actualbudget#1150)
  Removing Tutorial code (actualbudget#1146)
  Removing unused functions (actualbudget#1145)
  Revert “Make number parsing agnostic to decimal and thousands separators” (actualbudget#1144)
  Strip a trailing slash off of server URLs (actualbudget#1140)
  Update CONTRIBUTING.md to point to the website (actualbudget#1138)
  ...
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this pull request Mar 7, 2024
This PR is for the first item listed in actualbudget#559: `Expand All / Collapse All
Categories`

For the expand / collapse all categories functionality, I was choosing
between having a single `Expand / collapse all categories` button or one
for each: `Expand all categories` and `Collapse all categories` buttons.

For the initial implementation, I have opted with the latter. Please let
me know which one is the right way to go or if there are other
suggestions and I'll just accordingly.


![image](https://github.com/actualbudget/actual/assets/20313680/64d0e498-1139-4dd0-9b7f-4d478ab947aa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Merged Pull Request has been merged successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants