-
Notifications
You must be signed in to change notification settings - Fork 188
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
chore(generative-ai): move ai sign in modal to gen ai package from atlas-service COMPASS-8459 #6448
Changes from all commits
91bfbc5
b75899d
a5cd319
404c18b
813ed8e
458dad5
d1514f8
7bc3e60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,6 @@ | ||
import { ipcRenderer } from 'hadron-ipc'; | ||
import type { CompassAuthService as AtlasServiceMain } from './main'; | ||
import { | ||
signInWithModalPrompt, | ||
signInWithoutPrompt, | ||
} from './store/atlas-signin-reducer'; | ||
import { signInWithoutPrompt } from './store/atlas-signin-reducer'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to above, this naming should probably be adjusted, no point in calling out that there is no prompt if there is no other option in this service |
||
import { getStore } from './store/atlas-signin-store'; | ||
import { AtlasAuthService } from './atlas-auth-service'; | ||
import type { ArgsWithSignal, SignInPrompt } from './atlas-auth-service'; | ||
|
@@ -46,8 +43,6 @@ export class CompassAtlasAuthService extends AtlasAuthService { | |
switch (promptType) { | ||
case 'none': | ||
return getStore().dispatch(signInWithoutPrompt({ signal })); | ||
case 'ai-promo-modal': | ||
return getStore().dispatch(signInWithModalPrompt({ signal })); | ||
default: | ||
return this.ipc.signIn({ signal }); | ||
} | ||
|
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.
We probably should consider how we can remote this type altogether now, I know that it's right now here so that the reducer can trigger the ipc without recursively calling itself, but this is probably more of a a flaw than a feature now that we don't have the second option
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.
Good callout, it could use an intentional function for each I feel. It's a bit overloaded without benefit at the moment. I'll see what we can do here.