Home Page Updates #5961
Home Page Updates #5961
Conversation
nickgrato
commented
Feb 23, 2023
•
nickgrato
commented
- Adding "--legacy-peer-deps" to npm ci because lilypad uses React 18
- Converting home page to functional react component and TS
- Adding new content to home page ( icons copy etc)
- Adding style library we use in marketing page and dashboard so all out utility and css vars are available. ( this is about 25 files, a majority of the PR. This is not brand new SCSS, it is approved code im copying over to be used and edit in the context of the admin.)
admin/types.d.ts
Outdated
| in_quota: boolean; | ||
| }; | ||
|
|
||
| export type ReticulumSceneListingT = { |
There was a problem hiding this comment.
@keianhzo do you know where a good place for me to look to build these types out?
There was a problem hiding this comment.
I believe that would be the reticulum source code:
- https://github.com/mozilla/reticulum/blob/master/lib/ret/meta.ex
- https://github.com/mozilla/reticulum/blob/master/lib/ret/avatar_listing.ex
- https://github.com/mozilla/reticulum/blob/master/lib/ret/scene_listing.ex
- https://github.com/mozilla/reticulum/blob/master/lib/ret/account.ex
- https://github.com/mozilla/reticulum/blob/master/lib/ret/storage.ex
There was a problem hiding this comment.
amazing thank you
There was a problem hiding this comment.
@keianhzo ok I think this is ready to review when you get a chance - it's a big departure from the current way we do things here so let me know if you want me to walk through my future plans if something seems odd.
There was a problem hiding this comment.
Nice to see how we are consolidating things here and in the dashboard. Looking good. Just left a few nits, mostly things that we were already doing in a weird way before so I don't want to block on this so take them as suggestions in case you want to look into them now or prefer to let them for the future.
| useEffect(() => { | ||
| const init = async () => { | ||
| try { | ||
| const adminInfo = await getAdminInfo(); |
There was a problem hiding this comment.
All this queries will only throw an exception in case of network failure but not when a valid HTTP response is received including 404 or 500. We are also acting differently for getAdminInfo vs getEditableConfig updateReticulumMeta where in the first one we catch the exception but we don't in the others. We will probably need to check the response "ok" field to see if the request was successful. Also as we are just logging when error, we can probably put them in the same ty/catch block.
| const needsScenes = reticulumMeta.repo && !reticulumMeta.repo.scene_listings.any; | ||
| const exceededStorageQuota = reticulumMeta.repo && !reticulumMeta.repo.storage.in_quota; | ||
| const isInSESSandbox = | ||
| adminInfo && adminInfo.using_ses && adminInfo.ses_max_24_hour_send <= MAX_AWS_SES_QUOTA_FOR_SANDBOX; |
There was a problem hiding this comment.
Looking at the types it seems that if we get and AdminInfoT response it will always contain all the properties so not sure if it makes sense to check them all here. If not maybe they should be optional?
There was a problem hiding this comment.
Also al this state properties only seem to make sense if the async requests are successful so maybe it would make sense to include them in the same effect and initialize to false?
There was a problem hiding this comment.
check what types are optional for this.
| retConfig.phx && | ||
| retConfig.phx.cors_proxy_url_host === `cors-proxy.${adminInfo.worker_domain}`; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
Would it make sense to show some error card in case the async requests fail?
There was a problem hiding this comment.
putting a todo note for this!
| </Card> | ||
|
|
||
| <div className="flex-align-items-center ml-12"> | ||
| <a className="link mr-24">What's new</a> |
There was a problem hiding this comment.
This was already like that but it doesn't seem to go anywhere
| @@ -0,0 +1,41 @@ | |||
| export type ReticulumMetaT = { | |||
There was a problem hiding this comment.
@bryanenders @netpro2k Was hoping to get some insight on if any of these types are conditional so I can throw the "?" on them or if we think they are all expected every time?
