-
Notifications
You must be signed in to change notification settings - Fork 78
Feature request: Allow creation of new chats using query params #232 #234
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
base: main
Are you sure you want to change the base?
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.
Some comments around extra code but otherwise LGTM
| selectedChatModel: serverPreferences.selectedChatModel, | ||
| imageGenerationModel: serverPreferences.imageGenerationModel, | ||
| useNativeSearch: serverPreferences.useNativeSearch, | ||
| useNativeSearch: prefillQuery ? true : serverPreferences.useNativeSearch, // Enable web search when query is provided |
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 there a need for this?
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.
Looking back at it now, i think this might be redundant now too, since web search is auto enabled based on model capabilities, so no need to actually specify this.
I originally did this because the issue specifically requested enabling web search as well.
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.
if web search is auto-enabled, that's correct yeah, this is redundant! I wasnt sure
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.
What is the need for this page?
Seems like we can keep the query param behavior in the index route
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.
Yes, we can, but the route was requested, that's why I created it, also following the pattern of I think two out of the three examples that were provided uses it.
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.
This was part of the scope for the issue #232, to give it parity with the other providers and to reduce confusion about landing page vs. authed route, but maybe we dont need a /new?
| isNew, | ||
| workbench, | ||
| prefillQuery, | ||
| autoSubmitQuery, |
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 autoSubmitQuery ever used? Feels to me like if prefillQuery is defined it should autosubmit as the user has been linked into that
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.
Yes, without it the query just sits in the box. I included it last, it was the last request of the issue. #232
Implements the feature request from #232 to allow creation of new chats using query parameters.
Usage
/new- Creates a new empty chat/new?q=What%20is%20React%3F- Creates new chat with "What is React?" pre-filled and auto-executedcloses #232