feat: Dashboard Safe Apps #3738
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
|
I'd suggest merging |
DiogoSoaress
force-pushed
the
epic-dashboard
branch
from
8dd80ee
to
2e08cd6
Compare
DiogoSoaress
force-pushed
the
dashboard-safeapps
branch
from
1167b11
to
24f4199
Compare
| <div> | ||
| <h2>Safe Apps</h2> | ||
| {isLoading ? ( | ||
| <h2>Loading...</h2> |
There was a problem hiding this comment.
Good enough for now, but we should ideally display a card skeleton here instead. So that the height of the widget doesn't jump.
DiogoSoaress
force-pushed
the
dashboard-safeapps
branch
from
7e3fcd2
to
20fdeb6
Compare
ESLint Summary View Full Report
Report generated by eslint-plus-action |
DiogoSoaress
force-pushed
the
dashboard-safeapps
branch
from
20fdeb6
to
73aa39c
Compare
| @@ -75,6 +79,8 @@ export const ReviewConfirm = ({ | |||
| const nativeCurrency = getNativeCurrency() | |||
| const explorerUrl = getExplorerInfo(safeAddress) | |||
| const isOwner = useSelector(grantedSelector) | |||
| const { remoteSafeApps } = useRemoteSafeApps() | |||
There was a problem hiding this comment.
You don't need this. The modal takes an app prop that has an id.
There was a problem hiding this comment.
That id is not the id of type number that we are looking for. It is something of the type {"url":"https://dapp.daohaus.club/","name":"DAOhaus"}
as SafeApp type is https://github.com/gnosis/safe-react/blob/dda7f419599661d828165c111bbe58506c2ac5ca/src/routes/safe/components/Apps/types.ts#L4-L9
There was a problem hiding this comment.
The number id is overwritten
There was a problem hiding this comment.
What is the difference in the ids? Can we not just reference the 'new' id instead then?
There was a problem hiding this comment.
This is the overwritten id
That id is not the id of type number that we are looking for. It is something of the type {"url":"https://dapp.daohaus.club/","name":"DAOhaus"}
There was a problem hiding this comment.
Can we then pass the right id as a prop? I find it weird that the modal should fetch the list of apps again just to find an id.
There was a problem hiding this comment.
I have misunderstood your comment @katspaugh !! It makes all sense that we don't fetch the remote apps again so I prop drilled the appId from <AppFrame /> . 👍
src/routes/safe/components/Apps/components/ConfirmTxModal/ReviewConfirm.tsx
Outdated
Show resolved
Hide resolved
| const bTimeMultiplier = | ||
| b[1].timestamp.valueOf() - a[1].timestamp.valueOf() > 0 ? MORE_RECENT_MULTIPLIER : LESS_RECENT_MULTIPLIER | ||
|
|
||
| // The sorting score is a weighted function where the OPEN_C0UNT weights differently than the TX_COUNT |
There was a problem hiding this comment.
OPEN_C0UNT is spelled with a zero for some reason :)
| let currentTxCount = trackData[app.id]?.txCount | ||
| local.setItem(APPS_DASHBOARD, { | ||
| ...trackData, | ||
| [app.id]: { ...trackData[app.id], txCount: currentTxCount ? ++currentTxCount : 1 }, |
There was a problem hiding this comment.
Here and on line 22, it's a bit hard to read. I would suggest to write it like this:
const currentTxCount = trackData[app.id]?.txCount || 0
...
[app.id]: { ...trackData[app.id], txCount: currentTxCount + 1 }
You don't need to modify the variable, so no need for ++.
DiogoSoaress
force-pushed
the
dashboard-safeapps
branch
from
5f73012
to
c76c2a8
Compare
| {/* Bookmark button */} | ||
| <IconBtn onClick={handlePinClick}>{localPinned ? <Bookmark /> : <BookmarkBorder />}</IconBtn> | ||
|
|
||
| {/* TODO: Share button */} |
There was a problem hiding this comment.
Per discussion:
sharing is not enabled yet by Safe Apps, so no need to tackle it here already.
I will remove the comment.
|
|
||
| local.setItem(APPS_DASHBOARD, { | ||
| ...trackData, | ||
| [id]: { ...trackData[id], txCount: currentTxCount + 1 }, |
There was a problem hiding this comment.
I assume it will always have an openCount here as the App has opened?
What it solves
Resolves #3698
How this PR fixes it
Implements a "Safe Apps" section in the Dashboard with 5 SafeApps and a button to the Safe Apps page.
The first Safe Apps displayed are the most relevant by usage and are followed by random Safe Apps to fill the 5 Cards.
How to test it
Go to
/home.The "Safe Apps" section should show the ranked apps + random apps.
Screenshots