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

Use Actions for sortorder dropdown #736

Merged
merged 1 commit into from Jan 29, 2020
Merged

Use Actions for sortorder dropdown #736

merged 1 commit into from Jan 29, 2020

Conversation

raimund-schluessler
Copy link
Member

@raimund-schluessler raimund-schluessler commented Nov 26, 2019

This PR uses the nextcloud/vue Actions for the sortorder dropdown.

sortorder-dark

sortorder-light

@codecov
Copy link

codecov bot commented Nov 26, 2019

Codecov Report

Merging #736 into master will increase coverage by 0.23%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master     #736      +/-   ##
==========================================
+ Coverage   26.63%   26.87%   +0.23%     
==========================================
  Files          44       44              
  Lines        2140     2125      -15     
  Branches      395      391       -4     
==========================================
+ Hits          570      571       +1     
+ Misses       1445     1430      -15     
+ Partials      125      124       -1

@raimund-schluessler
Copy link
Member Author

@skjnldsv and @jancborchardt for the new sort-order icons.

Also, I had to use menu-align="right" for the Actions component, otherwise the dropdown would have been overlapped by the details panel. @skjnldsv Is this known and a non-issue or should I file a bug in nextcloud-vue?

@skjnldsv
Copy link
Member

Could you post screenshots with English 🙈
I don't really understand the right/left arrows.
And the alphabetically sort should either say ZA and have the triangle towards the bottom or AZ with the triangle towards the top, but like here it looks like sorting alphabetically will be descending instead of ascending ;)

@raimund-schluessler
Copy link
Member Author

raimund-schluessler commented Nov 27, 2019

Could you post screenshots with English 🙈

Sorry, of course, where are my manners 😄 I updated the screenshots.

I don't really understand the right/left arrows.

They are supposed to indicate start (left) and due date (right). I was thinking about the scroll forward and backward buttons on media players, but I am open for other ideas.

And the alphabetically sort should either say ZA and have the triangle towards the bottom or AZ with the triangle towards the top, but like here it looks like sorting alphabetically will be descending instead of ascending ;)

You are right, the arrows were reversed. I adjusted it to comply with how we do it in the files app. The letters are now also ZA for reverse sort.

@raimund-schluessler
Copy link
Member Author

Ping @jancborchardt and @skjnldsv and maybe @nextcloud/designers (sorry for the noise if you are not interested).

@skjnldsv
Copy link
Member

skjnldsv commented Dec 2, 2019

I think the arrow could use better iconography :)

@jancborchardt
Copy link
Member

Due date and start date could use some sort of calendar icon. Start date could be the calendar icon, and due date a flag maybe.

But I don’t think the icons are the real issue here. :) The main problem is that there are a huge mass of sorting options:

  • 8 ways of sorting (!?)
  • Each way has a reverse sorting way as well?
  • What is "Default"?

How about having only 1 proper way of doing it?

  • Since it is a Tasks app, the order should always be by priority. We could also have inbetween headings for that.
  • Then if something is due soon, that should be sorted up top of course.
  • Among the same priority level, things are sorted by last modified cause that indicates activity and interest. Also helps to see what others worked on in shared task lists.
  • Completed tasks are sorted by completion date, most recently completed up top – and of course completed tasks are below all the other tasks, right?

Also:

  • Sorting tasks with a free text input by alphabet doesn’t make so much sense. Reverse alphabetically for sure not.
  • Last modified is a better version of "Created date", so we don’t need the latter.
  • How can we factor in the "Start date" here, or do we even need to?
  • If this is simplified as described above, we can totally cut the "Default" option which I assume is set in the bottom left settings.

@raimund-schluessler
Copy link
Member Author

@jancborchardt Thanks for your feedback, but limiting / cleaning-up the sorting options is not what this PR is about 😉

Due date and start date could use some sort of calendar icon. Start date could be the calendar icon, and due date a flag maybe.

This sounds good, I will adjust it. Do we have a flag somewhere?

But I don’t think the icons are the real issue here. :) The main problem is that there are a huge mass of sorting options:

  • 8 ways of sorting (!?)
  • Each way has a reverse sorting way as well?
  • What is "Default"?

They all where requested at some point, and I don't really see a point in limiting the sorting options. They are just entries in a list, every entry is explained with a tooltip, e.g. "Sort by completed state, due date, priority, start date and summary." for the "Default" order (https://github.com/nextcloud/tasks/blob/master/src/components/SortorderDropdown.vue#L73) and there is nothing to configure really. Also, sorting the tasks by different criteria is a main purpose of this app. And I bet if we limit the options, we sooner or later get reports asking where it went 😉 So, to make it short, I would like to keep the sorting options. 🙂

How about having only 1 proper way of doing it?

  • Since it is a Tasks app, the order should always be by priority. We could also have inbetween headings for that.
  • Then if something is due soon, that should be sorted up top of course.
  • Among the same priority level, things are sorted by last modified cause that indicates activity and interest. Also helps to see what others worked on in shared task lists.
  • Completed tasks are sorted by completion date, most recently completed up top – and of course completed tasks are below all the other tasks, right?

This is pretty much the "Default" sort order, which is currently completed>due>priority>start>alphabetically. I am fine to change it as you proposed to completed>due>priority>last-modified, though.

  • Sorting tasks with a free text input by alphabet doesn’t make so much sense. Reverse alphabetically for sure not.
  • Last modified is a better version of "Created date", so we don’t need the latter.

Depends on how you use the app, I guess.

  • If this is simplified as described above, we can totally cut the "Default" option which I assume is set in the bottom left settings.

There is no option to chose which one is the default order, the selected one is simply saved to the server and restored on app load. "Default" might not be the best naming, since it implies there would be an option to chose. We could call it "General" or "Standard".

@jancborchardt
Copy link
Member

jancborchardt commented Dec 3, 2019

This is pretty much the "Default" sort order, which is currently completed>due>priority>start>alphabetically. I am fine to change it as you proposed to completed>due>priority>last-modified, though.

There is no option to chose which one is the default order, the selected one is simply saved to the server and restored on app load. "Default" might not be the best naming, since it implies there would be an option to chose. We could call it "General" or "Standard".

Ah cool, yes, this clears it up! :) Good question how to call that sorting indeed. Maybe "Relevance"?

Although:

completed>due>priority>last-modified

Completed tasks are sorted above all? Should be below all, no?


They all where requested at some point, and I don't really see a point in limiting the sorting options.

Yes, of course everything gets requested all the time in an open source project. ;)
It’s not so much about limiting, but about really finding out why something is needed. E.g. maybe the "Default" sorting was not there in the beginning? And the limiting is also useful because when you focus on the essentials the whole thing becomes less confusing and imposing → friendlier and easier to use.


As for the icons, calendar for "Due date" and icon-confirm for "Start date" seems good.

@raimund-schluessler
Copy link
Member Author

Ah cool, yes, this clears it up! :) Good question how to call that sorting indeed. Maybe "Relevance"?

Relevance sounds good, let's use this.

completed>due>priority>last-modified

Completed tasks are sorted above all? Should be below all, no?

Completed tasks are below all others. This just meant that we sort by completed status first (completed below), then we sort tasks with the same completed status by due date (first due top), then tasks with same due date are sorted by priority, etc.

Yes, of course everything gets requested all the time in an open source project. ;)
It’s not so much about limiting, but about really finding out why something is needed. E.g. maybe the "Default" sorting was not there in the beginning? And the limiting is also useful because when you focus on the essentials the whole thing becomes less confusing and imposing → friendlier and easier to use.

I see your point, I just think that having eight options how to sort the tasks is not confusing, since it is about the same thing (sorting). But let's discuss this in a separate issue, I will open one.

As for the icons, calendar for "Due date" and icon-confirm for "Start date" seems good.

Sounds good, I will adjust the PR.

@raimund-schluessler
Copy link
Member Author

raimund-schluessler commented Dec 31, 2019

As for the icons, calendar for "Due date" and icon-confirm for "Start date" seems good.

@jancborchardt Do you really mean icon-confirm for start date? The icon-confirm arrow points to the right. Since we read from left (start) to right (end), it seems more appropriate for the due date (end date). Do you mind if I mirror the arrow to make it point to the left when using it for the start date?

@jancborchardt
Copy link
Member

@raimund-schluessler yes, I'd say use the icon-confirm. The arrow icon signifies "go"/"start", no need for additional complexity. (Mirroring it for example makes it into a "Back" icon.)

@zeroepix
Copy link

I don't really understand the right/left arrows.

They are supposed to indicate start (left) and due date (right). I was thinking about the scroll forward > and backward buttons on media players, but I am open for other ideas.

Intuitively start date should be something like |-> rather than |<-
Like you're running a race: |-> start (run run run) finish ->|

@raimund-schluessler
Copy link
Member Author

After thinking about it, I agree. Also the icons you find when searching for "due start date icon" mostly look like this. But I would combine it with the calendar icon. So for start date the arrow starts in the center of the calender, for end date it ends in the center.

I think this has a number of advantages over the icon-confirm / icon-calendar combination:

  • it is clear, that both icons belong together
  • we don't misuse icon-confirm for a not related topic
  • we can use them in the details sidebar (icon-confirm look quite off there)
  • the icon-due with arrow would be different from the icon-calendar we use for week in the app-navigation, so it is clear, this is not the same

I will adjust the PR and let you have a look.

@raimund-schluessler
Copy link
Member Author

I updated the PR, see #736 (comment) for the screenshots. I think it looks fine like this and better than icon-confirm. What do you think?

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

At least in my humble opinion, they look great! The more I look at them the more I like them, they're all distinct enough to be able to tell them from a glance (so you don't have to waste time reading). Good stuff.

@raimund-schluessler
Copy link
Member Author

Ping @jancborchardt and @skjnldsv.

@raimund-schluessler raimund-schluessler merged commit d8a26c7 into master Jan 29, 2020
@delete-merged-branch delete-merged-branch bot deleted the sortorder branch January 29, 2020 07:57
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