[Feature] Implementing Draggable Cards | Dashboard#3069
[Feature] Implementing Draggable Cards | Dashboard#3069Civolilah wants to merge 14 commits intoinvoiceninja:developfrom
Conversation
|
@Civolilah looks good, for now, could we please remove the refresh button, otherwise good to merge. |
@Civolilah i think there is an issue with the labelling here. 
We need to include sum,average,count labels as well. |
|
@turbo124 The logic for refreshing data in cards has been commented out for now, and the labeling has also been improved. Screenshot: 
Let me know your thoughts. |
| return; | ||
| } | ||
|
|
||
| const updated = cloneDeep(user) as User; |
There was a problem hiding this comment.
Unnecessary as. Type should be inferred.
| ).then((response: GenericSingleResourceResponse<CompanyUser>) => { | ||
| set(updated, 'company_user', response.data.data); | ||
|
|
||
| $refetch(['company_users']); | ||
|
|
||
| dispatch(updateUser(updated)); | ||
| dispatch(resetChanges()); | ||
| }); |
There was a problem hiding this comment.
Do we not need handling for errors here?
There was a problem hiding this comment.
@beganovich Hmm, I don't think so. We get the response from the API with the saved data and simply update the Redux state with it. I mean, we do the same thing in any other place where we need to update Redux, so I don't think we need it.
| useEffect(() => { | ||
| setOrder(currentDashboardFields); | ||
| }, [currentDashboardFields.length]); |
There was a problem hiding this comment.
You can potentially get rid of this effect by top-down re-render key.
// In parent (Totals.tsx)
<PreferenceCardsGrid
key={currentDashboardFields.join(',')} // Remount
currentDashboardFields={currentDashboardFields}
// ...
/>
Please test this and see if it works.There was a problem hiding this comment.
@beganovich Yes, we can achieve it on this way also. It works fine.
| }, [manageOpen, currentUser]); | ||
|
|
||
| const handleSave = () => { | ||
| const updated = cloneDeep(currentUser) as User; |
There was a problem hiding this comment.
Unnecessary type assertion again, should be inferred.
3 participants
@beganovich @turbo124 The PR includes the implementation of total cards on the dashboard. Screenshot:
Let me know your thoughts.