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

Add the selected contacts to a group #2763

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MrPompom
Copy link
Contributor

Linked to the pull request #2756 (comment)

By using the menu of the selected contacts.

Add selected contacts in a group who already exist

When the multiselect is clicked, it will display all the group which already exists and by clicking on a group, it will add all the selected contacts to this group

image

Create a group and add the selected contacts

It's the same but when typing the name of the new group and press down the return key, it will create a group of this name and add the selected contacts into it

image

@szaimen szaimen requested review from st3iny and GretaD June 18, 2022 15:39
@szaimen szaimen added the 3. to review Waiting for reviews label Jun 18, 2022
@codecov
Copy link

codecov bot commented Jun 18, 2022

Codecov Report

Base: 67.96% // Head: 67.96% // No change to project coverage 👍

Coverage data is based on head (4d49713) compared to base (e32f12d).
Patch has no changes to coverable lines.

❗ Current head 4d49713 differs from pull request most recent head 15e64b5. Consider uploading reports for the commit 15e64b5 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2763   +/-   ##
=========================================
  Coverage     67.96%   67.96%           
  Complexity      253      253           
=========================================
  Files            23       23           
  Lines           721      721           
=========================================
  Hits            490      490           
  Misses          231      231           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@szaimen szaimen requested review from st3iny and removed request for st3iny October 4, 2022 23:45
@szaimen szaimen force-pushed the add-to-group-the-selected-contats branch from 82fb27f to f7a792c Compare October 11, 2022 10:06
@szaimen
Copy link
Contributor

szaimen commented Oct 11, 2022

I just rebased this. Hopefully I didn't break anything while doing so.

@szaimen szaimen force-pushed the add-to-group-the-selected-contats branch 2 times, most recently from 01631a5 to e2559bd Compare October 17, 2022 19:48
MrPompom and others added 4 commits October 17, 2022 21:49
Add checkbox on hover and add a button to delete the selected contacts
Signed-off-by: Bastien PROMPSY <[email protected]>
Add a button to merge the selected contacts

Signed-off-by: Bastien PROMPSY <[email protected]>
When the button 'add to group' is clicked, a multiselect appears who allow to select the group or type it

Signed-off-by: Bastien PROMPSY <[email protected]>
Signed-off-by: szaimen <[email protected]>
@szaimen szaimen force-pushed the add-to-group-the-selected-contats branch from e2559bd to 15e64b5 Compare October 17, 2022 19:49
@szaimen
Copy link
Contributor

szaimen commented Nov 8, 2022

@ChristophWurst I fixed the imports :)

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

First look

menu-align="right">
<template v-if="selected.length >= 1">
<ActionButton v-if="showAddToGroup"
icon="icon-clone"
Copy link
Member

Choose a reason for hiding this comment

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

icon classes have been deprecated. please use a material icon.

</template>
<ActionButton v-if="selected.length >= 2"
:close-after-click="true"
icon="icon-clone"
Copy link
Member

Choose a reason for hiding this comment

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

material icon required

</ActionButton>
<ActionButton v-if="selected.length >= 1"
:close-after-click="true"
icon="icon-delete"
Copy link
Member

Choose a reason for hiding this comment

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

material icon required

</div>
<VirtualList ref="scroller"
class="contacts-list"
data-key="key"
:data-sources="filteredList"
:data-component="ContactsListItem"
:estimate-size="68" />
:estimate-size="68"
:extra-props={selected}
Copy link
Member

Choose a reason for hiding this comment

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

missing quotes


export default {
name: 'ContactsList',

components: {
AppContentList,
VirtualList,
Actions,
ActionButton,
Multiselect
Copy link
Member

Choose a reason for hiding this comment

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

missing trailing comma

const contactjcal = this.contacts[element].jCal[1]
this.addValue(firstContact, contactjcal)
// delete the contact merged and the uid in the selected
this.$store.dispatch('deleteContact', { contact: this.contacts[element] }) && this.selected.splice(this.selected.indexOf(element), 1)
Copy link
Member

Choose a reason for hiding this comment

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

await the action

})
this.$store.dispatch('updateContact', firstContact)
},
OpenMultiselect() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
OpenMultiselect() {
openMultiselect() {

camel case for methods

const selectedContact = this.contacts[element]
const data = selectedContact.groups
if (!data.includes(value)) {
this.$store.dispatch('addContactToGroup', {
Copy link
Member

Choose a reason for hiding this comment

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

await the action

isUnselected() {
this.ischecked = false
// update the selected
this.$parent.$parent.$emit('update-check-selected', this.source.key)
Copy link
Member

Choose a reason for hiding this comment

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

accessing the grandparent is a no go. this makes the component depend on the exact structure and is prone to break in unnoticed ways. Please find a better way to pass around the data. Could you use Vuex?

isSelected() {
this.ischecked = true
// update the selected
this.$parent.$parent.$emit('update-check-selected', this.source.key)
Copy link
Member

Choose a reason for hiding this comment

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

please don't access the grandparent component this way

@ChristophWurst ChristophWurst marked this pull request as draft April 20, 2023 08:31
@ChristophWurst ChristophWurst added 0. to triage Pending approval or rejection. This issue is pending approval. and removed 3. to review Waiting for reviews labels Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. to triage Pending approval or rejection. This issue is pending approval.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants