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

introduce done and open filters #5488

Merged

Conversation

grnd-alt
Copy link
Member

@grnd-alt grnd-alt commented Jan 17, 2024

Summary

introduces two new filters.
done filter: shows only cards that are done on the board
open filter: shows only cards that are open on the board

Screenshot:
image

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

@stefan-niedermann
Copy link
Member

Would you mind adding a screenshot for easier review? 🙂

@stefan-niedermann
Copy link
Member

stefan-niedermann commented Jan 17, 2024

Thank you very much, that's pretty much what I expected it from a look into the source code.

But the approach with the radio buttons has one issue: Once set, only all filter can be cleared.

Therefore two UX questions:

  1. Is filtering for Done actually necessary? If not we could simply go for one checkbox or switch Hide done
  2. If it is absolutely necessary, maybe a toggle button group of three buttons would make sense? Undone | All | Done

Personally I also would merge it with the due date section as these information play together, also in the cards themself.

Last not least; Wording. In the cards itself the action is called Completed, it should be the same everywhere. Mixing up the terms Completed and Done is confusing.

@juliusknorr
Copy link
Member

Thanks for the contribution. The wording is indeed not 100% consistent also in the existing code base, I'd settle on "Completed" and "Not completed".

Is filtering for Done actually necessary? If not we could simply go for one checkbox or switch Hide done

I'd say showing all, completed, uncompleted is still usful, so a threeway option sounds good 👍

@grnd-alt Could you adapt that and also check the failing eslint job and DCO Then this seems good to get merged

@grnd-alt
Copy link
Member Author

@juliushaertl
By a threeway option, do you mean having three radio buttons like the existing ones?
Or rather having a new type of filter option with multiple buttons in one row?

@juliusknorr
Copy link
Member

juliusknorr commented Jan 19, 2024

By a threeway option, do you mean having three radio buttons like the existing ones?

I think for the matter of this PR this is the most sensible option.

Or rather having a new type of filter option with multiple buttons in one row?

This sounds like generally a nicer idea but the best way to implement this might be to migrate the full menu to the NcActions component and then use NcActionButtonGroup/NcActionCheckbox/NcActionRadio for the list entries. So if you're eager feel free to dive into that but I would not consider that required for the scope of this issue for now.

@grnd-alt
Copy link
Member Author

Ok, great! I'll finish this PR up, and possibly start a refactoring in another PR/issue.

@pedroponte

This comment has been minimized.

@grnd-alt grnd-alt force-pushed the enhancement/introduce-done-open-filters branch 3 times, most recently from d73bbbc to 39ed175 Compare January 31, 2024 19:04
@juliusknorr juliusknorr self-requested a review February 14, 2024 11:14
@juliusknorr
Copy link
Member

Sorry for the delay on reviewing, will check again today or tomorrow

Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long, I left some working suggestions and a small comment to simplify the code a bit

src/components/Controls.vue Outdated Show resolved Hide resolved
src/components/Controls.vue Outdated Show resolved Hide resolved
src/components/Controls.vue Outdated Show resolved Hide resolved
src/components/Controls.vue Outdated Show resolved Hide resolved
Signed-off-by: grnd-alt <[email protected]>
Signed-off-by: grnd-alt <[email protected]>
Signed-off-by: grnd-alt <[email protected]>
Signed-off-by: grnd-alt <[email protected]>
refactor element-ids to filter-option-

Signed-off-by: grnd-alt <[email protected]>
@grnd-alt grnd-alt force-pushed the enhancement/introduce-done-open-filters branch from 39ed175 to c6728d3 Compare March 13, 2024 14:44
@juliusknorr juliusknorr merged commit 2c6f81a into nextcloud:main Mar 21, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide marked as done cards
4 participants