Skip to content

Conversation

@juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented May 16, 2017

Signed-off-by: Julius Härtl jus@bitgrid.net

  • get board permissions on boardlist
  • Apply archive filter to the sidebar
  • Add unit tests

@artemanufrij
Copy link
Member

@juliushaertl It's not possible to move a board from archive into standard view. After click on "..." board opens.

@artemanufrij
Copy link
Member

artemanufrij commented May 17, 2017

@juliushaertl after moving board into archive it stays in the navigation bar. I guess it should be removed from navigationbar...

@codecov
Copy link

codecov bot commented May 18, 2017

Codecov Report

Merging #133 into master will increase coverage by 3.78%.
The diff coverage is 66.93%.

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
+ Coverage   72.34%   76.12%   +3.78%     
==========================================
  Files          32       33       +1     
  Lines         846      955     +109     
==========================================
+ Hits          612      727     +115     
+ Misses        234      228       -6

@juliusknorr juliusknorr added this to the 0.2.0 milestone May 18, 2017
@juliusknorr
Copy link
Member Author

@artemanufrij This is ready to test. Only unit tests are missing for some new methods.

@artemanufrij artemanufrij self-requested a review May 18, 2017 10:52
Copy link
Member

@artemanufrij artemanufrij left a comment

Choose a reason for hiding this comment

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

  1. after unarchive a board deck stays in "archive"-view but shows only "unarchived" boards.
  2. It shouldn't be possible to create a new board in archive-view

@juliusknorr juliusknorr force-pushed the archive-boards branch 2 times, most recently from 9e5caaf to eef2b7c Compare May 28, 2017 13:16
@artemanufrij
Copy link
Member

position of context menu is broken:
screenshot_20170528_153029

@juliusknorr juliusknorr force-pushed the archive-boards branch 7 times, most recently from ffdd615 to 5a20b04 Compare May 29, 2017 07:55
Copy link
Member

@artemanufrij artemanufrij left a comment

Choose a reason for hiding this comment

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

some comments:

  • it's not possible to remove a board in default view
  • deleting doesn't work in archive view
    PS: restore works
  • position of context menu is broken (see screenshot) PS: style.css has conflicts

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
artemanufrij and others added 3 commits June 8, 2017 20:52
Signed-off-by: Artem Anufrij <artem.anufrij@live.de>
Signed-off-by: Artem Anufrij <artem.anufrij@live.de>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr juliusknorr requested a review from pixelipo June 8, 2017 19:03
@juliusknorr juliusknorr changed the title WIP: Archive boards Archive boards Jun 8, 2017
@juliusknorr
Copy link
Member Author

@artemanufrij @pixelipo Please review.

This also fixes #85

@artemanufrij
Copy link
Member

artemanufrij commented Jun 8, 2017

@juliushaertl
Undo works fine for board but not for card or stack. --> it's ok for me

Board will be permanent deleted after 5 minutes.

Copy link
Member

@artemanufrij artemanufrij left a comment

Choose a reason for hiding this comment

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

works as expected (discussed in irc-channel)...
Test cases:

  • move board into archive 👍
  • move board from archive into default view 👍
  • delete board 👍
  • undo deleting operation 👍

@juliusknorr
Copy link
Member Author

Thanks for testing 👍 I'll merge this now so we have it in todays nightly builds for further testing.

@juliusknorr juliusknorr merged commit 48842dd into master Jun 8, 2017
@juliusknorr juliusknorr deleted the archive-boards branch June 8, 2017 20:46
}

.app-navigation-entry-utils-menu-button {
display: block !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can safely be deleted

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

*/

.app-navigation-entry-menu ul {
flex-direction: row;
Copy link
Contributor

Choose a reason for hiding this comment

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

According to @jancborchardt icons should be stacked vertically (with a label next to them):

#121 (comment)

padding-left: 4px;
border-radius:3px;
flex-direction: row;
min-width: 240px;
Copy link
Contributor

Choose a reason for hiding this comment

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

can it be larger anyway? Maybe width: 100%; would make more sense here.

Edit: It can, in the table. However, the question then is should it really grow? Setting a fixed width at 240px and removing flex-grow on line 720 would make both colorlists equal in size.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about that, since we have 2 places with different width using this.

2017-06-08-230340_1155x200_scrot

*/

.colorselect {
overflow: hidden;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a case where this container overflows?

Copy link
Member Author

@juliusknorr juliusknorr Jun 8, 2017

Choose a reason for hiding this comment

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

no, i think that is a leftover from using floats before using flexbox with this PR.

}

.colorselect .selected {
background-image: url(../../../core/img/actions/checkmark.svg);
Copy link
Contributor

Choose a reason for hiding this comment

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

It owuld be better to move his to a new class icon-selected or icon-checkmark. Then it can be reused. Also, next two lines could be removed.

P.S. do we need ../../..? on my setup(s), url(/core/img/actions/checkmarks.svg) works fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

/core/ will not work on setups where nextcloud is running inside of a subdirectory, so the ../../../ are needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

but classes like icon-selected and icon-selected-dark makes sense.

}

.colorselect .dark.selected {
background-image: url(../../../core/img/actions/checkmark-white.svg);
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

position: relative;
}

.popovermenu ul {
Copy link
Contributor

Choose a reason for hiding this comment

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

This style already exists in core css that is served alongside, so perhaps it;s not needed?

.bubble ul, .app-navigation-entry-menu ul, .popovermenu ul {
    display: flex !important;
    flex-direction: column;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 But i need to check that.

width: 50%;
}

#boardlist tr.deleted td * {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just change the opacity of the whole row with #boardlist tr.deleted { ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It will also change the opacity of the border that separates the lines, that looked a bit odd. 😉

@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 512 512"><path d="M432.3 448.1h-348c-13.2 0-24-10.8-24-24V264.6c0-13.2 10.8-24 24-24h348c13.2 0 24 10.8 24 24v159.5c0 13.2-10.8 24-24 24zM380.4 89.8H127.8c-7.7 0-14-6.3-14-14v-6.3c0-7.7 6.3-14 14-14h252.6c7.7 0 14 6.3 14 14v6.3c0 7.7-6.3 14-14 14zm19.4 61.8H110.6c-7.7 0-14-6.3-14-14v-6.3c0-7.7 6.3-14 14-14h289.2c7.7 0 14 6.3 14 14v6.3c0 7.7-6.3 14-14 14zm21.6 61.4H94.6c-7.7 0-14-6.3-14-14v-6.3c0-7.7 6.3-14 14-14h326.8c7.7 0 14 6.3 14 14v6.3c0 7.7-6.3 14-14 14z"/></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to keep this in line with the other Nextcloud icons. I will commit an optimized version of the icon to this PR - I hope you don't mind 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

Since you merged before my review, I created a new PR #175

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. feel free to open a new PR, since this one is already merged 🙈

@juliusknorr
Copy link
Member Author

@pixelipo Thanks for the comments on the css changes. I'll try to address those in a follow up PR tomorrow.

@jancborchardt
Copy link
Member

Awesome work by you all on the Deck app by the way! :) I want to check it out more too, and I would actually use it for my Tasks if it would use the Nextcloud dav backend (so I can simply switch between Deck and the Tasks app whenever I like :)

@juliusknorr
Copy link
Member Author

@jancborchardt Thanks for checking it out :) Integration is also one of my main goals but until now I have not come up with a satisfying solution on how to integrate it with the dav backend. I'll look deeper into this after the next version is released.

@LunaUrsa
Copy link

I appreciate all the work done here. Is there any thought about making Decks archivable the way Boards and Slides can be?

@juliusknorr
Copy link
Member Author

@TeknoShamin Yes, I think archiving stacks would make sense as well. Can you open a new issue for that?

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.

6 participants