add a chatbox example #205
Conversation
|
This looks pretty awesome! Thanks for the PR 🙏 I’m away until the 11th of July. I’ll check out the example for myself (across a few different platforms) and review the code once I get back. Just from a quick look through the code, I’d suggest using a NavigationSplitView to make the sidebar (so that it’s resizable and adapts to mobile). Also, you can use |
There was a problem hiding this comment.
I'm back from holidays now. I've left quite a few comments throughout the PR, mostly related to things that I believe could be significantly improved with suitable refactoring. Sorry if it's quite a bit; the SwiftCrossUI examples are the first serious SwiftCrossUI app codebases that many developers will see if they start looking into SwiftCrossUI, so I hold them to a pretty high standard. Let me know if any of the comments don't make sense or could do with elaboration!
The main overarching changes that I'd like to see are:
- the ability to use the app without providing an OpenAI API key (by way of a simple demo mode), and
- the ability to use the app on non-Apple platforms (which is currently prevented by use of UserDefaults)
Purely out of interest, did you use AI while producing this code?
| // MARK: - Error Types | ||
|
|
There was a problem hiding this comment.
Remove the MARKs (just a codebase style preference)
| case 401: | ||
| return "API Error 401: Invalid API key. Please check your OpenAI API key and make sure it's valid and has sufficient credits." | ||
| case 429: | ||
| return "API Error 429: Rate limit exceeded. Please wait a moment and try again." |
There was a problem hiding this comment.
Remove the error code from the errors with nice human readable errors. E.g. the 401 error should read "Invalid API key...".
Keep default case and the server error case as they are because the error codes might carry more information.
| // MARK: - Thread Models | ||
|
|
||
| struct ChatThread: Identifiable, Codable { | ||
| let id: String |
There was a problem hiding this comment.
Make the id a UUID like you did for ChatMessage.
| // MARK: - Thread Message | ||
|
|
||
| struct ThreadMessage: Identifiable, Codable { | ||
| let id: String |
There was a problem hiding this comment.
Make the id a UUID like you did for ChatMessage.
|
|
||
| // MARK: - Thread Message | ||
|
|
||
| struct ThreadMessage: Identifiable, Codable { |
|
|
||
| // MARK: - Chat Header View | ||
|
|
||
| struct ChatHeaderView: View { |
There was a problem hiding this comment.
You can move the bodies of these small views into the MainChatView instead by creating computed properties, similar to var body, like so:
@ViewBuilder
var header: some View {
HStack { ... }
}| VStack(alignment: .leading, spacing: AppSpacing.xs) { | ||
| Text(message.content) | ||
| .font(AppFonts.body) | ||
| .foregroundColor(AppColors.text) | ||
| .padding(AppSpacing.md) | ||
| .background(AppColors.surface) | ||
| .cornerRadius(AppCornerRadius.large) | ||
| .frame(maxWidth: 600, alignment: .leading) | ||
|
|
||
| Text(formatTime(message.timestamp)) | ||
| .font(AppFonts.caption2) | ||
| .foregroundColor(AppColors.textSecondary) | ||
| } |
There was a problem hiding this comment.
This is repeated from the user version above. Extract the logic for converting a message to a view into a seperate function like so:
func renderMessage(_ message: ChatMessage) -> some View {
VStack(...) { ... }
}| ThreadRowView( | ||
| thread: thread, | ||
| isSelected: selectedThread?.id == thread.id, | ||
| onSelect: { onSelectThread(thread) }, |
There was a problem hiding this comment.
I believe that you can just do onSelect: onSelectThread to make this a bit cleaner. Same for onDelete.
| .fontWeight(.semibold) | ||
| .foregroundColor(AppColors.text) | ||
|
|
||
| Text("Start a new conversation to\nbegin chatting with AI!") |
There was a problem hiding this comment.
Try to avoid manually wrapping the string if possible. Let me know if this has to do with weird line wrapping behaviour or something though.
| WindowGroup("ChatBot") { | ||
| #hotReloadable { | ||
| NavigationSplitView { | ||
| // Sidebar content |
There was a problem hiding this comment.
Remove these section comments from this app body. The parts are all relatively self explanatory already imo.
Uh oh!
There was an error while loading. Please reload this page.