Added support for task pinning

[timholl]

Implements #694

[codecov]

Codecov Report

Merging #695 into master will increase coverage by 0.25%.
The diff coverage is 21.21%.

@@            Coverage Diff             @@
##           master     #695      +/-   ##
==========================================
+ Coverage   10.89%   11.14%   +0.25%     
==========================================
  Files          44       44              
  Lines        2020     2045      +25     
  Branches      360      366       +6     
==========================================
+ Hits          220      228       +8     
- Misses       1668     1683      +15     
- Partials      132      134       +2

[timholl]

Pinned state can be toggled by clicking on the pin icon next to the star

Pinned tasks also get an additional icon in the list view

[raimund-schluessler]

Thanks for the contribution.

Regarding the feature:
This feature has no effect in the web interface. Toggling the pinned state does not change anything and only has an effect for users of the OpenTasks app. Considering this, the implementation is quite prominent in the web interface. I wonder whether we can realise this a bit more unobtrusively. @nextcloud/designers @jancborchardt Any opinion?

Regarding the code:
It looks very good and clean 👍 You only need to signoff your commit, see https://github.com/nextcloud/tasks/pull/695/checks?check_run_id=282263610

[raimund-schluessler]

We could also try to add some benefit for the web interface. E.g. show a pinned task always at the top of a list.

[timholl]

We could also try to add some benefit for the web interface. E.g. show a pinned task always at the top of a list.

Sounds like a good idea to me, implemented that prematurely.

[timholl]

To justify myself a litte for https://github.com/nextcloud/tasks/pull/695/files#diff-88cbe2e916273c5c244516d0ec6b33ffR756-R757 :

I had a little struggle with the icons since I added two and as a side-effect the big star icon (and the new pin) did not work anymore.
I figured out that was because the sprite was resized from 5x5 to 6x5.
No problem for svg-sprite module to find the correct positions in CSS, but the 1.5-upscaling of the background for .star.icon-bw class was hardcoded in the CSS for 5x5 (to 120px 120px) and also for .star.icon-color. Had to take that numbers apart and put them humpty-dumpty back together for 6x5 (kinda "improved" it by using auto for the second parameter so there is just one hardcoded number left).

Maybe one could find a more conceptual solution for this in the future than incrementing a hardcoded number ^^

[raimund-schluessler]

See my two comments, I think we are good after that.

[raimund-schluessler]

To justify myself a litte for https://github.com/nextcloud/tasks/pull/695/files#diff-88cbe2e916273c5c244516d0ec6b33ffR756-R757 :

I had a little struggle with the icons since I added two and as a side-effect the big star icon (and the new pin) did not work anymore.
I figured out that was because the sprite was resized from 5x5 to 6x5.
No problem for svg-sprite module to find the correct positions in CSS, but the 1.5-upscaling of the background for .star.icon-bw class was hardcoded in the CSS for 5x5 (to 120px 120px) and also for .star.icon-color. Had to take that numbers apart and put them humpty-dumpty back together for 6x5 (kinda "improved" it by using auto for the second parameter so there is just one hardcoded number left).

Maybe one could find a more conceptual solution for this in the future than incrementing a hardcoded number ^^

Don't worry, I will have a look at this in a follow-up PR. We will move the header to flex-box then this should not be necessary anymore.

[raimund-schluessler]

I had a little struggle with the icons since I added two and as a side-effect the big star icon (and the new pin) did not work anymore.
I figured out that was because the sprite was resized from 5x5 to 6x5.
No problem for svg-sprite module to find the correct positions in CSS, but the 1.5-upscaling of the background for .star.icon-bw class was hardcoded in the CSS for 5x5 (to 120px 120px) and also for .star.icon-color. Had to take that numbers apart and put them humpty-dumpty back together for 6x5 (kinda "improved" it by using auto for the second parameter so there is just one hardcoded number left).

I only now got what you meant. This is indeed not really nice, but I couldn't find any other solution so far. And obviously, flex-box won't do much about it.

[raimund-schluessler]

I tend to reduce the pin size down to a normal icon (16px x 16px), but let's discuss this in a later PR.