-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#307 new page to display recent messages #375
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
name={`${item.isAssistant ? "Assitant" : "User"}`} | ||
src={`${item.isAssistant ? "/logos/logo.png" : "/images/temp-avatars/av1.jpg"}`} | ||
/> | ||
<Box className={`p-4 rounded-md text-white whitespace-pre-wrap ${bgColor} text-white w-full`}>{item.text}</Box> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this work in dark mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
borderRadius="xl" | ||
className="p-6 shadow-sm" | ||
> | ||
{isLoading ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please move the isLoading
check to the parent component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Screen.Recording.2023-01-04.at.20.05.16.mov
sure, i just had it in there to show small spinners... but could also change it to show a loading screen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now removed... but still looks the same :)
method: "GET", | ||
headers: { | ||
"X-API-Key": process.env.FASTAPI_KEY, | ||
"Content-Type": "application/json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you arguably don't need content-type of a GET because it has no body.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed it
website/src/pages/messages/index.tsx
Outdated
); | ||
}; | ||
|
||
MessagesDashboard.getLayout = (page) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this layout used somewhere else? I remember seeing something similar, if you find a similar layout please move it to its own component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is removed
Can you make the padding between the title and div consistent with the widgets on the dashboard? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like all the other requested changes have been made. I approve but won't merge until @AbdBarho gets a chance to look again.
spacing for message dashboard (comment in #375)
Initial implementation for #307.

Looks like this now:
Unfortunately the flagging currently does not work..