Skip to content

Comments

WIP: New CardModal UI#3557

Closed
luka-nextcloud wants to merge 16 commits intomainfrom
feature/update-card-modal-ui
Closed

WIP: New CardModal UI#3557
luka-nextcloud wants to merge 16 commits intomainfrom
feature/update-card-modal-ui

Conversation

@luka-nextcloud
Copy link
Contributor

@luka-nextcloud luka-nextcloud commented Jan 25, 2022

Signed-off-by: Luka Trovic luka@nextcloud.com

Summary

TODO

  • ...

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

Signed-off-by: Luka Trovic <luka@nextcloud.com>
Signed-off-by: Luka Trovic <luka@nextcloud.com>
@luka-nextcloud
Copy link
Contributor Author

Members tab:
image

Signed-off-by: Luka Trovic <luka@nextcloud.com>
@luka-nextcloud
Copy link
Contributor Author

image

@luka-nextcloud luka-nextcloud changed the title New CardModal UI WIP: New CardModal UI Jan 31, 2022
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

I didn't check the code yet.

  • The CardModal component is not displayed in a modal but rather at the right sidebar's location.
  • The member/user multiselect disappears on mouseout.
  • I don't see the tag input like in your screenshot.

Did I do something wrong? Did you forgot to include some commits?

Signed-off-by: Luka Trovic <luka@nextcloud.com>
Signed-off-by: Luka Trovic <luka@nextcloud.com>
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.

Looks fine from the overall structure, I left some comments inline and also a longer one regarding the "tabs" behaviour which might be something to adjust further.

Comment on lines 27 to 30
Example task 3
</h1>
<p class="top-modified">
Modified: 2 days go. Created 3 days ago
Copy link
Member

Choose a reason for hiding this comment

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

Should of course use the actual card data instead of placeholder text.

<div class="tabs">
<div class="tab members" :class="{active: activeTab === 'members'}" @click="activeTab = 'members'">
<i class="icon-user icon" />
Members
Copy link
Member

Choose a reason for hiding this comment

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

Please make this translatable

</div>
<div class="tab tags" :class="{active: activeTab === 'tags'}" @click="activeTab = 'tags'">
<i class="icon icon-tag" />
Tags
Copy link
Member

Choose a reason for hiding this comment

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

Please make this translatable

</div>
<div class="tab due-date" :class="{active: activeTab === 'duedate'}" @click="activeTab = 'duedate'">
<i class="icon icon-calendar-dark" />
Due date
Copy link
Member

Choose a reason for hiding this comment

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

Please make this translatable

</div>
<div class="tab project" :class="{active: activeTab === 'project'}" @click="activeTab = 'project'">
<i class="icon icon-deck" />
Project
Copy link
Member

Choose a reason for hiding this comment

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

Please make this translatable

</div>
<div class="tab attachments">
<i class="icon-attach icon icon-attach-dark" />
Attachments
Copy link
Member

Choose a reason for hiding this comment

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

Please make this translatable

</div>
<div class="activities">
<h2 class="activities-title">
<div class="icon-flash-black" /> Activities
Copy link
Member

Choose a reason for hiding this comment

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

I'd call this icon-activity then, and the text should be translatable

<Avatar :user="currentUser.uid" />
<CommentForm v-model="newComment" @submit="createComment" />
</div>
<div class="activities-logs">
Copy link
Member

Choose a reason for hiding this comment

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

Activities and comments are two different things, we currently have no way to merge them easily on the backend side, so in addition to this section. So for adding the activities list, we can make use of the already existing component from https://github.com/nextcloud/deck/blob/master/src/components/card/CardSidebarTabActivity.vue#L24-L29

Copy link
Member

Choose a reason for hiding this comment

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

For comments itself I think we should then also reuse the existing component for now:

https://github.com/nextcloud/deck/blob/master/src/components/card/CardSidebarTabComments.vue

Basically other than in the mockups, lets go for something like this to start with for the UI structure of the new modal:

Comments
----------
CardSidebarTabComments

Activities
----------
CardSidebarTabActivity

Modified: 2 days go. Created 3 days ago
</p>
</div>
<div class="tabs">
Copy link
Member

Choose a reason for hiding this comment

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

I think there might have been a bit of confusion on what the purpose of the top buttons is, they are not tabs in specific, but more toggle buttons for the actual input fields. Let me maybe describe the idea there a bit further:

Taking the due date as an example

  • By default the input field is hidden
  • The user decides to add a due date by clicking the "Due date" button
  • Then the input is shown
  • If a card has a due date set, we always show the input field

The same would be the case for the others (Members, Projects, Attachments, Tags)

@@ -0,0 +1 @@
<svg fill="#26e07f" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24px" height="24px"><path fill-rule="evenodd" d="M 11 2 L 11 11 L 2 11 L 2 13 L 11 13 L 11 22 L 13 22 L 13 13 L 22 13 L 22 11 L 13 11 L 13 2 Z"/></svg>
Copy link
Member

Choose a reason for hiding this comment

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

This should be available already as a global icon class icon-add https://docs.nextcloud.com/server/latest/developer_manual/html_css_design/icons.html

Otherwise we can make use of https://github.com/robcresswell/vue-material-design-icons for pulling in icons.

Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

Ok got it, "Use bigger card view" must be checked to display the card modal.

It looks nice!

  • If the modal content is too long, there is no way to scroll down. A simple way to solve that is to add overflow: scroll; to the .modal-container but there might be a cleaner approach.
  • We could use more partial vue component import like in line 48 of App.vue
  • Clicking on tabs does not have any effect on the modal content. What's the purpose of the tabs?
  • User select, tag, date and "Add to project" are all displayed on the same line
    aaa
  • Multiselect disappears on mouseout for users and tags

Signed-off-by: Luka Trovic <luka@nextcloud.com>
Signed-off-by: Luka Trovic <luka@nextcloud.com>
Signed-off-by: Luka Trovic <luka@nextcloud.com>
@luka-nextcloud
Copy link
Contributor Author

@juliushaertl @max-nextcloud Could you review? Thanks.

Signed-off-by: Luka Trovic <luka@nextcloud.com>
@juliusknorr
Copy link
Member

Let me close this since this has quite some heavy conflicts meanwhile, we can pick this up once relevant again

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.

Improve modal view design

3 participants