-
Notifications
You must be signed in to change notification settings - Fork 31
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
Feat/add multiselection #940
Conversation
And review the initialization of flags in the app
In order to be able to take advantage of the latest rules of `eslint-config-cozy-app`.
Which generated a warning in console
And split action to specific file
behind `select-all-contacts` flag
BundleMonFiles updated (5)
Unchanged files (2)
Total files change +9.89KB +0.77% Groups updated (2)
Unchanged groups (1)
Final result: ✅ View report in BundleMon website ➡️ |
@@ -72,7 +74,8 @@ export const ContentResult = ({ contacts, allGroups }) => { | |||
searchValue | |||
) | |||
setFilteredContacts(filteredContactsBySearch) | |||
}, [contacts, searchValue, selectedGroup, setFilteredContacts]) | |||
setContactsDisplayed(filteredContactsBySearch) | |||
}, [contacts, searchValue, selectedGroup, setContactsDisplayed]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between filteredContacts and ContactsDisplayed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aucune, si ce n'est que ContactsDisplayed
est accessible pour tous les composants (ContactsSelectionBar
aujourd'hui)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dans ce cas, pourquoi en créer un nouveau ? On devrait rendre accessible ce ContactsDisplayed
partout, non ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setFilteredContacts
est un état local déjà présent et surtout initialisé avec les contacts traités par son parent.
De part l'archi actuelle, le remplacer ici par le contexte ContactsDisplayed
, fait apparaitre un instant le composant ContactsEmptyList
(même problème en utilisant le contexte dans le parent)
Je n'ai pas de solution viable rapide, et je dois reprendre d'autres développements, mais à l'écoute si jamais !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je valide pour avancer, mais ça me dérange fortement cette duplication.
c505a81
to
76f3c02
Compare
This context is useful to keep in memory the contacts displayed, this will allow for example to select only these, because they could be filtered beforehand.
And use `queryAll` instead `query`
76f3c02
to
2c121aa
Compare
Ad add option parameter
2c121aa
to
7e13be9
Compare
And removed the use of react-redux because it's not used, and is no longer relevant.
We want to offer the choice to delete the contacts associated with the group that we want to delete.
And removed the old approach which became useless
7e13be9
to
b770db6
Compare
@Crash-- Retours effectués 👍 |
SelectionBar
)