-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
added modal to select source documentation #103
Conversation
added modal to select source documentation
small fixes
small nav fix
Since docs are being fetched from the network. We should show something to represent loading state when Docs modal is launched in case of delay. Can be done in next PR. |
frontend/src/api/docs.ts
Outdated
} catch (error) { | ||
return null; | ||
} | ||
} |
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.
Since functionality in this file is unique to preference feature, it could be moved inside preference
folder.
Something like this : https://github.com/arc53/DocsGPT/blob/main/frontend/src/conversation/conversationApi.ts
frontend/src/models/misc.ts
Outdated
dat: string; | ||
docLink: string; | ||
model: string; | ||
}; |
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.
Since this type is unique to preference feature, it could be moved inside preference folder as well.
|
||
const isSelectedDocsSet = useSelector(selectSelectedDocsStatus); | ||
const [selectedDocsModalState, setSelectedDocsModalState] = | ||
useState<ActiveState>(isSelectedDocsSet ? 'INACTIVE' : 'ACTIVE'); |
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 gotta put the two modal opening in sequence, can do it in the next PR.
isCancellable?: boolean; | ||
}) { | ||
const dispatch = useDispatch(); | ||
const [docs, setDocs] = useState<Doc[] | null>(null); |
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.
I am on the fence about where to put all of the docs collection inside component state or redux. On one hand it feels like its okay as long as curently selected doc is in redux. On other hand, it feels like next upcoming feature(upload #105) would require all docs to be in redux so that new doc could be added when user decides to upload a doc . Ill let @TaylorS15 and @dartpain handle this one.
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.
Agreed, because of upcoming feature i think its best if its in the redux
Everything has been changed per comments except for navigation opening the modals in sequence. |
Moved /api/docs.ts to /preference/selectDocsApi.ts Added all source docs to redux
review changes
little change
removed logging
added api/docs.ts to fetch default source docs
added new reducer to preferenceSlice
added button in navigation to open modal