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

Properly show all tasks matching a collection #431

Merged
merged 31 commits into from Nov 14, 2019

Conversation

raimund-schluessler
Copy link
Member

This PR aims at solving an issue with the current way we sort tasks into collections (see e.g #83). At the moment we only show a task in a collection if it is a root task and matches the collection in question.

E.g. we only show a task in the today collection when it is due today and if it is a root task. In case a subtask is due today, it will not be shown unless its root task is also due today.

The problem here is, that you potentially won't see subtasks due today / this week, are important. This means the collections don't catch all tasks that they should.

With this PR, we show all root tasks that have a subtask matching the collection (or match the collection itself).

The question here is, whether this is reasonable and how we should count the tasks then (for showing their count in the sidebar). Options are to

  • only count root tasks (currently done)
  • or to count all tasks directly matching the collection)

Feedback from @nextcloud/designers would be very welcome.

@jancborchardt
Copy link
Member

Do you have screenshots of a scenario? That would be much appreciated to illustrate the issue & solution. :)

@raimund-schluessler
Copy link
Member Author

raimund-schluessler commented May 29, 2019

Do you have screenshots of a scenario? That would be much appreciated to illustrate the issue & solution. :)

This is an example for the starred collection which shows important tasks, but it works for the other collections just the same way.

Before this PR the starred collection looks like this:
Screenshot_2019-05-29 Aufgaben - Nextcloud(1)
It shows two important tasks, which are shown because they are important and root tasks. What it does not show is that there are two more important tasks which are subtasks of an other task.

With this PR, also important subtasks show up in the starred collection:
Screenshot_2019-05-29 Aufgaben - Nextcloud
Now the important subtasks lead to the task Test 5 being shown, also showing the important subtasks. I think, this is the better behaviour, since you now really see all important tasks (subtasks or not).

What I would propose to additionally add is a counter showing the number of sub(sub..)tasks matching this collection (e.g. in a colored circle). So the task Test 5 will have a counter 2 , the task Test 5 - Subtask 1 will show 2 as well and Test 5 - Subtask 1 - Subtask 2 will show 1.

@raimund-schluessler
Copy link
Member Author

One more question would be now, what the counter in the app-sidebar shows. It could show 3 as it does at the moment with this PR (second screenshot), countin all root tasks. Or it could show the actual number of important tasks, namely 4 in that case.

@skjnldsv
Copy link
Member

I would say (if I got this right), for the task 5, always show the full task tree if it leads to a starred task.
But don't show any sibling/children tasks that does not contain nor are favourited :)

@raimund-schluessler
Copy link
Member Author

I would say (if I got this right), for the task 5, always show the full task tree if it leads to a starred task.
But don't show any sibling/children tasks that does not contain nor are favourited :)

That would be an option as well.

@jancborchardt
Copy link
Member

Hmm, so when you favorited a child task at the 3rd level deep, why not just show only that? That would simplify the whole structure a lot and not make the tree structure necessary. Viewing favorites is about focusing, and the structure might detract.

Otherwise we can do it like you and @skjnldsv proposed – show the structure for reference, but leave out siblings which are not favorited.

@raimund-schluessler
Copy link
Member Author

I think not showing the full tree might be confusing. You expect a subtask to be shown as a subtask.

I will implement it as @skjnldsv proposed it. Show the tree if it leads to a starred subtask. I will then show the number of other (not shown) subtasks as a number with a tooltip. When you open the details of this task showing all subtasks then might be good as well.

@codecov
Copy link

codecov bot commented Oct 26, 2019

Codecov Report

Merging #431 into master will increase coverage by 15.32%.
The diff coverage is 82.02%.

@@             Coverage Diff             @@
##           master     #431       +/-   ##
===========================================
+ Coverage   11.13%   26.46%   +15.32%     
===========================================
  Files          44       44               
  Lines        2047     2086       +39     
  Branches      367      386       +19     
===========================================
+ Hits          228      552      +324     
+ Misses       1685     1417      -268     
+ Partials      134      117       -17

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Only show subtasks in a collection
when they or a descendant task match the collection

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Not only root tasks

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler
Copy link
Member Author

raimund-schluessler commented Nov 12, 2019

Now that it can happen that a task is shown multiple times in the week view (e.g. a task a two subtasks due today and tomorrow) a task should only be shown as active if it is the very exact task.

  • If a task is opened on day 1, don't also show it open on day 2.
  • Only show not matching subtasks for active tasks
  • Show parent tasks if a descendant is open

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler
Copy link
Member Author

@skjnldsv @jancborchardt This was quite a bit of work, but I think it is finally done.

With this PR all tasks (including subtasks) are shown if they match a collection. We show the full tree for subtasks matching the collection, but don't show siblings which don't match. When a task is open in the details view, all its subtasks are shown.

For the starred view this looks like this.

No task open in details view:
Screenshot_2019-11-14 Aufgaben - Nextcloud(3)

Task which is not matching the collection is open (showing all its subtasks):
Screenshot_2019-11-14 Aufgaben - Nextcloud

Subtask which doesn't match the collection is open (still showing full tree to this subtask):
Screenshot_2019-11-14 Aufgaben - Nextcloud(2)

I also added a lot of tests to check for the correct behaviour of the collections.

What is still left to be done is to indicate which task exactly matches a collection and which one is only shown because a subtask matches. I would like to do this in a follow up PR, because this one is already quite huge.

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@@ -35,7 +35,18 @@ const routes = [
// reliably with router-link due to
Copy link
Member

Choose a reason for hiding this comment

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

the router file is usually located into /src/router/index.js ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I would say it's fine as well in a router.js file, more clear ;)

I like having everything in separate folders, but feel free to choose :)
Both are fine imho (just not in components 😉 )

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

See one unrelated comment! :)

@raimund-schluessler raimund-schluessler merged commit 511502a into master Nov 14, 2019
@raimund-schluessler raimund-schluessler deleted the fix-collections branch November 14, 2019 19:35
@vozeldr
Copy link

vozeldr commented Dec 19, 2019

I'm using 0.11.3 on 17.0.1 and this is still an issue, so it doesn't seem like it fixed the problem. Creating a subtask with a due date when the parent does not have a due date does not show that task in Important, Today, or Week.

@raimund-schluessler
Copy link
Member Author

This fix is not included in 0.11.3. It will be released with 0.12.0.

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.

None yet

4 participants