feat(dashboard): Nv 5276 Subscriber UI overview tab #7632
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
BiswaViraj
changed the title
…d add loading skeleton component
BiswaViraj
force-pushed
the
nv-5276-ui-overview-tab
branch
from
4d24095 to
7e45fa7
Compare
BiswaViraj
requested review from
SokratisVidros,
LetItRock and
ChmaraX
and removed request for
SokratisVidros
| /> | ||
| <SheetPortal> | ||
| <SheetContentBase asChild onInteractOutside={handleCloseSheet} onEscapeKeyDown={handleCloseSheet}> | ||
| <motion.div |
There was a problem hiding this comment.
Since this Sheet is reusable across multiple screens (Create workflows, create environment, etc...) the appearance and exit animation should be part of the Sheet and not redefined every time we use it.
| </PopoverTrigger> | ||
| <PopoverContent className="w-[300px] p-0"> | ||
| <Command> | ||
| <CommandInput placeholder="Search country..." /> |
There was a problem hiding this comment.
IMO, this dropdown needs a bit of polishing around the paddings, borders on focus, and the search input; please talk to Naveen about how we can improve it.
| <SubscriberOverviewForm | ||
| subscriberId={subscriberId} | ||
| subscriber={isFetching ? undefined : data} | ||
| isFetching={isFetching} |
There was a problem hiding this comment.
isPending is always false on subsequent requests so isFetching is appropriate in our usecase
There was a problem hiding this comment.
You want it to show the skeleton again after saving? This makes no sense. Save has finished, data will be the same as the data you typed, and you show a skeleton.
There was a problem hiding this comment.
No, it is not for that scenario,
it is for showing skeleton when switching subscribers, with isPending only the first subscriber shows the skeleton, but when you switch to a different subscriber, isPending remains false, because it still has the old subscriber data
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer