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

Clone boards with stacks, labels #1221

Merged
merged 5 commits into from
Oct 10, 2019
Merged

Clone boards with stacks, labels #1221

merged 5 commits into from
Oct 10, 2019

Conversation

jakobroehrl
Copy link
Contributor

@jakobroehrl jakobroehrl commented Sep 6, 2019

Signed-off-by: Jakob <jakob.roehrl@web.de>
@juliusknorr
Copy link
Member

Is there a good Icon I can use?

We could use this, but maybe that is a better fit for the move action instead of the clone.
image

But maybe we should just create a simple icon similar to this one:
https://thenounproject.com/search/?q=clone&i=2432924

cc @nextcloud/designers

@skjnldsv
Copy link
Member

skjnldsv commented Sep 7, 2019

<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><path d="M11.8 13.8H2.2V4.2h9.6m1.2 0c0-.67-.53-1.2-1.2-1.2H2.2C1.53 3 1 3.53 1 4.2v9.6c0 .67.53 1.2 1.2 1.2h9.6c.67 0 1.2-.53 1.2-1.2"/><path d="M4.2 1C3.53 1 3 1.54 3 2.2v1.26h1.2V2.2h9.6v9.6h-1.14V13h1.14c.67 0 1.2-.53 1.2-1.2V2.2c0-.67-.53-1.2-1.2-1.2H4.2z"/></svg>

here you go, using the same design as the external icon.
external

@jakobroehrl
Copy link
Contributor Author

@juliushaertl
Added the Icon, but it is't shown. Can you have a look please?

@skjnldsv
Copy link
Member

skjnldsv commented Sep 10, 2019

@jakobroehrl please push your work :)

css/icons.scss Outdated Show resolved Hide resolved
@juliusknorr
Copy link
Member

Looks good in general, do you have debug enabled in your config.php? Otherwise the scss cacher might not update if the underlying scss file has changes.

@jancborchardt
Copy link
Member

<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16"><path d="M11.8 13.8H2.2V4.2h9.6m1.2 0c0-.67-.53-1.2-1.2-1.2H2.2C1.53 3 1 3.53 1 4.2v9.6c0 .67.53 1.2 1.2 1.2h9.6c.67 0 1.2-.53 1.2-1.2"/><path d="M4.2 1C3.53 1 3 1.54 3 2.2v1.26h1.2V2.2h9.6v9.6h-1.14V13h1.14c.67 0 1.2-.53 1.2-1.2V2.2c0-.67-.53-1.2-1.2-1.2H4.2z"/></svg>

here you go, using the same design as the external icon.
external

Improving the icon a bit, the shapes could be separated to make clear it’s separate, even in small sizes:

<svg width="16" height="16" version="1.1" xmlns="http://www.w3.org/2000/svg"><path d="M11.8 13.8H2.2V4.2h9.6m1.2 0c0-.67-.53-1.2-1.2-1.2H2.2C1.53 3 1 3.53 1 4.2v9.6c0 .67.53 1.2 1.2 1.2h9.6c.67 0 1.2-.53 1.2-1.2"/><path d="m4.2 1c-0.67 0-1.2 0.54-1.2 1.2h10.8v10.8c0.67 0 1.2-0.53 1.2-1.2v-9.6c0-0.67-0.53-1.2-1.2-1.2z"/></svg>

copy

This is in line with how we do it with other icons like external, icon-menu-sidebar, icon-audio-off and all the -off / -disabled state icons, and similar to how the Material Design copy icon is made. :)

img/clone.svg Outdated Show resolved Hide resolved
css/icons.scss Outdated Show resolved Hide resolved
img/clone.svg Outdated Show resolved Hide resolved
Signed-off-by: Jakob <jakob.roehrl@web.de>
@jakobroehrl
Copy link
Contributor Author

Now it should be fine

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.

Code looks good :)

@juliusknorr
Copy link
Member

So some small UX issues:

  • I think we should show a loading indicator until the clone has finished, since this might take some time
  • Ideally the new board starts in editing mode in the sidebar, so you can change the title right away

@juliusknorr
Copy link
Member

Besides that I'm fine with it 😉 Awesome work so far @jakobroehrl

@putt1ck putt1ck mentioned this pull request Sep 17, 2019
Signed-off-by: Jakob <jakob.roehrl@web.de>
@jakobroehrl
Copy link
Contributor Author

@juliushaertl
Loading: done
How can I get the boardId of the just created board?
newBoard here is empty:

this.$store.dispatch('cloneBoard', this.board).then((newBoard) => {

@juliusknorr
Copy link
Member

Loading: done
How can I get the boardId of the just created board?
newBoard here is empty:

Does your request return the data?

@jakobroehrl
Copy link
Contributor Author

How can I get the boardId of the just created board?
newBoard here is empty:

Does your request return the data?

@juliushaertl
Yes, I have the data here:

cloneBoard(state, board) {

But I need it here:

this.$store.dispatch('cloneBoard', this.board).then((newBoard) => {

skjnldsv
skjnldsv approved these changes Sep 20, 2019
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.

I'll let you fix what's left, once ready, please ping me again :)

@juliusknorr juliusknorr added this to the 🚀 Next major 1.0.0 milestone Oct 4, 2019
@juliusknorr juliusknorr changed the title clone board func with labels and stacks Clone boards with stacks, labels Oct 4, 2019
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.

Some remaining minor comments, otherwise it looks good 👍

css/icons.scss Show resolved Hide resolved
lib/Controller/BoardController.php Outdated Show resolved Hide resolved
lib/Service/BoardService.php Outdated Show resolved Hide resolved
lib/Service/BoardService.php Outdated Show resolved Hide resolved
src/store/main.js Outdated Show resolved Hide resolved
Signed-off-by: Jakob <jakob.roehrl@web.de>
src/services/BoardApi.js Outdated Show resolved Hide resolved
src/store/main.js Outdated Show resolved Hide resolved
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 comments :)

Signed-off-by: Jakob <jakob.roehrl@web.de>
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.

code looks good! 👍

@juliusknorr juliusknorr merged commit bb2eb72 into vue Oct 10, 2019
@juliusknorr juliusknorr deleted the boardDuplicate branch October 10, 2019 07:03
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.

4 participants