search

numerique-gouv / impress

MIT License
13 stars 7 forks source link

Lots of dynamic notification banners when I invite a couple people on the doc #246

Open virgile-dev opened 1 month ago

virgile-dev commented 1 month ago

Feature Request

Is your feature request related to a problem or unsupported use case? Please describe. If I invite a few people on the doc, one notification banner appears for each user. Which is annoying

Describe the solution you'd like Can we make a single notification for the whole group or just change the UX. Or better if invitations went through, just close the pane and notify me if there is an error.

PanchoutNathan commented 2 weeks ago

It has to come from the back first. Because it is impossible to add several members at once, we have to do a foreach. I think this is a pretty bad practice for several reasons. Like error handling or performance too. @sampaccoud @AntoLC

AntoLC commented 2 weeks ago

I think you can, when all the promises are made, we foreach them to show our toast popup, see: https://github.com/numerique-gouv/impress/blob/1d1832b67a2453fefb298c9faad926f7c6865599/src/frontend/apps/impress/src/features/docs/members/members-add/components/AddMembers.tsx#L137-L148

I think we should keep this mechanism for onError, it is nice to know with which member the endpoint failed, but when it is successful, to show only 1 toast is probably enough, moreover they are listed in real time in your member list.

PanchoutNathan commented 2 weeks ago

Yes, it is clearly possible on the front side not to display several toasts. But I find it a shame to do so many "tricks" when it would be enough for the back to accept a batch of members to be added in a single POST.

Because regarding performance or even simpler error management (because in the case where there are lots of errors, it amounts to displaying n toasts, or if we only display one error toast, we will not have the detail, except by tinkering)

But if you have to do the front version, no problem.