account for no visible assistants #2765
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces a new NoAssistantModal component and modifies the ChatPage to handle scenarios where no assistants are available in the chat interface.
- Added
/web/src/components/modals/NoAssistantModal.tsxto display different messages for admin and non-admin users when no assistants are available - Modified
/web/src/app/chat/ChatPage.tsxto implement logic for showing the NoAssistantModal when necessary - Updated
/web/src/components/BasicClickable.tsxto accept an optional 'className' prop for enhanced styling flexibility - Implemented conditional rendering in NoAssistantModal based on user admin status, providing appropriate guidance
3 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -139,7 +140,7 @@ export function ChatPage({ | |||
|
|
|||
| const [showApiKeyModal, setShowApiKeyModal] = useState(true); | |||
|
|
|||
| const { user, refreshUser, isLoadingUser } = useUser(); | |||
| const { user, refreshUser, isAdmin, isLoadingUser } = useUser(); | |||
There was a problem hiding this comment.
style: Consider destructuring isAdmin from the useUser hook to improve readability
| {showApiKeyModal && !shouldShowWelcomeModal ? ( | ||
| <ApiKeyModal | ||
| hide={() => setShowApiKeyModal(false)} | ||
| setPopup={setPopup} | ||
| /> | ||
| ) : ( | ||
| noAssistants && <NoAssistantModal isAdmin={isAdmin} /> |
There was a problem hiding this comment.
style: Consider extracting this conditional rendering logic into a separate function for better readability
| @@ -26,7 +28,9 @@ export function BasicClickable({ | |||
| select-none | |||
| overflow-hidden | |||
| hover:bg-hover-light | |||
| ${fullWidth ? "w-full" : ""}`} | |||
| ${fullWidth ? "w-full" : ""} | |||
| ${className ? className : ""} | |||
There was a problem hiding this comment.
style: consider using template literal for className to avoid potential undefined value
| onClick={() => { | ||
| window.location.href = "/admin/assistants"; | ||
| }} | ||
| className="inline-flex flex justify-center items-center px-4 py-2 border border-transparent text-sm font-medium rounded-md shadow-sm text-white bg-background-800 text-center focus:outline-none focus:ring-2 focus:ring-offset-2 " |
There was a problem hiding this comment.
syntax: remove duplicate 'flex' class
| window.location.href = "/admin/assistants"; | ||
| }} |
There was a problem hiding this comment.
style: consider using Next.js router for navigation instead of directly manipulating window.location
Description
[Provide a brief description of the changes in this PR]
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Accepted Risk
[Any know risks or failure modes to point out to reviewers]
Related Issue(s)
[If applicable, link to the issue(s) this PR addresses]
Checklist: