Skip to content
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

feat(opentrons-ai-client): add jotai and custom hook for call api #15029

Merged
merged 13 commits into from
May 3, 2024

Conversation

koji
Copy link
Contributor

@koji koji commented Apr 26, 2024

Overview

introduce a statement manage pacekage, jotai (https://jotai.org/)
The reasons I selected jotai are below.

  • easy to use (like useState + useContext) and learning cost wouldn't be high
  • this app won't need to a lot of statements in the future (I assumed we would need light mode/dark mode + chat history + α)
  • the document is maintained well
  • don't need to use provider but we can use provider to pass a specific statement to a specific component (really flexible)
  • easy to test https://jotai.org/docs/guides/testing
  • can use zustand if needed https://jotai.org/docs/extensions/zustand

add a function to call api (this will be update soon)

update ChatDisplay to align with the design.

markdown parsing part is waiting for the design team's update

app_test.mov

Test Plan

Changelog

Review requests

Risk assessment

low

koji added 4 commits April 25, 2024 15:50
add api call function

close AUTH-
…d fetch function

add jotai to make state management easy add fetch function

close AUTH-
@koji koji changed the title Feat add custom hook for call api feat(opentrons-ai-client): add jotai and custom hook for call api Apr 27, 2024
@@ -105,6 +105,7 @@
"handlebars-loader": "^1.7.1",
"html-webpack-plugin": "^3.2.0",
"identity-obj-proxy": "^3.0.0",
"jotai": "2.8.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to this since I cannot install the package without this.
I will dig into this issue to remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue was that I forgot to add opentrons-ai-client to workspace

@koji koji requested review from ncdiehl11 and jerader May 2, 2024 16:07
@koji koji marked this pull request as ready for review May 2, 2024 16:09
@koji koji requested a review from a team as a code owner May 2, 2024 16:09
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooh why do you need different favicon sizes? is it for different device sizes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we don't need to do this at this moment since the design doesn't support a smartphone and a tablet but we don't know what kind of devices user will use.

@@ -1,4 +1,5 @@
import React from 'react'
// import { css } from 'styled-components'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgot to delete this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will use this soon for adding style to markdown part.

},
query: prompt,
})
console.log('response', response)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgotten console log?

const userPrompt = watch('userPrompt') ?? ''
const [preparedPrompt] = useAtom(preparedPromptAtom)
const [, setChatData] = useAtom(chatDataAtom)
const [submitted, setSubmitted] = React.useState(false)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const [submitted, setSubmitted] = React.useState(false)
const [submitted, setSubmitted] = React.useState<boolean>(false)

const { userPrompt } = data
console.log('user prompt', userPrompt)
}
const [data, setData] = React.useState<any>(null)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data can be any?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is temporary type since we haven't started working on be.
Josh, Elyor and myself will start working on be and this type will be updated.

Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@koji koji merged commit b317cd4 into edge May 3, 2024
30 checks passed
@koji koji deleted the feat_add-custom-hook-for-callAPI branch May 3, 2024 17:42
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
…5029)

* feat(opentrons-ai-client): add jotai and custom hook for call api
Carlos-fernandez pushed a commit that referenced this pull request Jun 3, 2024
…5029)

* feat(opentrons-ai-client): add jotai and custom hook for call api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants