Open ai flyout #7

Samiul-TheSoccerFan · 2024-02-09T15:59:42Z

Summary

Added callout for Open AI key.
Added flyout when button clicked.
Saved the key into UI State

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Flaky Test Runner was used on any tests changed
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

yansavitski · 2024-02-09T16:03:19Z

packages/kbn-ai-playground/components/chat_sidebar.tsx

 import { i18n } from '@kbn/i18n';

 import { InstructionsField } from './instructions_field';
+import { OpenAIKeyCallout } from './open_ai_key_callout';
+import { OpenAIKeyFlyOut } from './open_ai_key_flyout';
+import { isEmpty } from 'lodash';

 export const ChatSidebar: React.FC = () => {


This component was deleted :(

yansavitski · 2024-02-09T16:06:38Z

packages/kbn-ai-playground/components/chat_sidebar.tsx

+        <OpenAIKeyFlyOut onClose={onCloseOpenAIFlyOut} setOpenAIKey={setOpenAIKey} />
+      ) : (
+        <>
+          {!isEmpty(openAIKey) ? (


Nitpick: could you please change condition just to improve readibility instead (not empty) ? not empty : not not empty use (is empty) ? empty : not empty

yansavitski · 2024-02-09T16:07:35Z

packages/kbn-ai-playground/components/chat_sidebar.tsx

-        value={prompt}
-        onChange={setPrompt}
-      />
+      {isOpenAIFlyOutOpen ? (


We don't to unmount other form components when we show flayout

yansavitski · 2024-02-09T16:12:57Z

packages/kbn-ai-playground/components/open_ai_key_callout.tsx

+import React, { Dispatch, SetStateAction } from "react";
+
+interface ListProps {
+  setIsOpenAIFlyOutOpen: Dispatch<SetStateAction<boolean>>;


nitpick: type is a bit odd, because in this component we like saying it's gonna recieve only action from state, but in reality it can be controlled in a different way and may change. So this component don't need to know about how it's gonna controlled only that function should get a boolean value

Probably as you gonna call only function to open better to just pass function like openFlyout withouth any params

yansavitski · 2024-02-09T16:14:45Z

packages/kbn-ai-playground/components/open_ai_key_callout.tsx

+  setIsOpenAIFlyOutOpen: Dispatch<SetStateAction<boolean>>;
+}
+
+export const OpenAIKeyCallout: React.FC<ListProps> = ({ setIsOpenAIFlyOutOpen }: ListProps) => {


nitpick: as it's a js arrow function and we don't use any body we can reduce code by remove return.

({ setIsOpenAIFlyOutOpen }: ListProps) => ( <EuiCallout ... /> )

I have chosen this format as we will need to add more code eventually. However, as we do not need now, I agree that we can remove it for now.

yansavitski · 2024-02-09T16:17:45Z

packages/kbn-ai-playground/components/open_ai_key_callout.tsx

+  return (
+    <EuiCallOut
+      css={css`
+        width: 55%


I suppose it should not be here
All offsets already settled in a parent block
We can check it together

yansavitski · 2024-02-09T16:22:22Z

packages/kbn-ai-playground/components/open_ai_key_flyout.tsx

+}
+
+export const OpenAIKeyFlyOut: React.FC<OpenAIKeyFlyOutProps> = ({ onClose, setOpenAIKey }: OpenAIKeyFlyOutProps) => {
+  const [apiKey, setApiKey] = useState<string>('');


I'm afraid we need to use useForm here, and check also isDirty value.
And allow to get props of default value for apiKey, because when it's open twice it suppose to be there
I can help you with that

yansavitski · 2024-02-09T16:24:39Z

packages/kbn-ai-playground/components/open_ai_key_flyout.tsx

+              isDisabled={!apiKey.trim()}
+              data-telemetry-id="entSearchAIPlayground-addingOpenAIKey-save"
+              fill
+              onClick={() => {


nitpick: let's create a function for it above because a lot of ui code and logic better to keep in place where it's easy to find. just readability issue

yansavitski · 2024-02-09T16:26:33Z

Can we have a component that will controll openAi behavior and add it to chat. So it will know when to show callout when to show edit text

yansavitski

LGTM
As improve of functionality we can add useForm to flyout though and extend some rules for save button

Samiul-TheSoccerFan · 2024-02-11T17:09:02Z

Hi @yansavitski , I tried to convert but got into another issue. For now, I am keeping it as it is and added a new task in the board https://github.com/elastic/enterprise-search-team/issues/6792 to tackle this later when we have a little down time (probably after the demo).

fixes [#8](elastic/observability-accessibility#8) fixes [#7](elastic/observability-accessibility#7) ## Summary Fixes APM breadcrumbs on serverless | Serverless | Stateful | |---|---| | <img width="700px" alt="image" src="https://github.com/user-attachments/assets/944a7d58-7de3-4a7f-be02-3c8c1110a0e2"> |<img width="800px" alt="image" src="https://github.com/user-attachments/assets/450664b1-ddfc-4395-9fa3-a7b941affb3b">| |<img width="500px" alt="image" src="https://github.com/user-attachments/assets/944a7d58-7de3-4a7f-be02-3c8c1110a0e2"> |<img width="500px" alt="image" src="https://github.com/user-attachments/assets/450664b1-ddfc-4395-9fa3-a7b941affb3b">| | <img width="500px" alt="image" src="https://github.com/user-attachments/assets/944a7d58-7de3-4a7f-be02-3c8c1110a0e2"> |<img width="500px" alt="image" src="https://github.com/user-attachments/assets/cb8a39e2-ca33-4cf9-a8ac-4c84566d092d">| |<img width="500px" alt="image" src="https://github.com/user-attachments/assets/151a3a9c-c81e-4558-9d00-e695e3d1d79c">|<img width="500px" alt="image" src="https://github.com/user-attachments/assets/2562e96f-d5e4-4aa4-a221-6721f8995883">| |<img width="500px" alt="image" src="https://github.com/user-attachments/assets/8d877d11-8c3f-4ac5-8146-6a11125eae7c">|<img width="500px" alt="image" src="https://github.com/user-attachments/assets/36e588cb-4c18-4d66-a2c6-f0e66392f708">| |<img width="500px" alt="image" src="https://github.com/user-attachments/assets/14253196-06de-4343-811f-61aa31ea0d1e">|<img width="500px" alt="image" src="https://github.com/user-attachments/assets/0cdfc83f-6545-433f-8c14-5bbf2a581175">| |<img width="500px" alt="image" src="https://github.com/user-attachments/assets/89a58e2b-2cef-4188-b2be-f359ba6890db">|<img width="500px" alt="image" src="https://github.com/user-attachments/assets/f15e767f-5b60-4485-ac71-7b6fd850ec50">| |<img width="500px" alt="image" src="https://github.com/user-attachments/assets/a0f7bfae-bfda-4f49-b92a-e736d80fea4c">|<img width="500px" alt="image" src="https://github.com/user-attachments/assets/680db8ab-58b8-454b-a0d7-6e1681dbe616">| ### How to test #### Serverless - Start a local ES serverless instance: `yarn es serverless --projectType=oblt --ssl -k/--insecure` - Start a local Kibana serverless instance: ` yarn start --serverless=oblt --no-ssl` - Run some synthtrace scenarios - `NODE_TLS_REJECT_UNAUTHORIZED=0 node scripts/synthtrace mobile.ts --live --target=https://elastic_serverless:[email protected]:9200 --kibana=http://elastic_serverless:[email protected]:5601` - `NODE_TLS_REJECT_UNAUTHORIZED=0 node scripts/synthtrace service_map.ts --live --target=https://elastic_serverless:[email protected]:9200 --kibana=http://elastic_serverless:[email protected]:5601` - Navigate to Applications and click through the links ### Stateful - Start a local ES and Kibana instance - Run the some synthtrace scenarios: - `node scripts/synthtrace mobile.ts --live` - `node scripts/synthtrace service_map.ts --live` - Navigate to Applications and click through the links --------- Co-authored-by: kibanamachine <[email protected]>

Samiul-TheSoccerFan changed the base branch from main to ai-playground February 9, 2024 16:00

yansavitski reviewed Feb 9, 2024

View reviewed changes

Samiul-TheSoccerFan added 2 commits February 9, 2024 11:54

Adding callout and Flyout component for Open AI Key

f52cdaf

refactored code to use Form hooks

a8a187b

Samiul-TheSoccerFan force-pushed the open-ai-flyout branch from bba50db to a8a187b Compare February 9, 2024 22:05

remove chat_sidebar

9df4444

yansavitski approved these changes Feb 9, 2024

View reviewed changes

yansavitski merged commit e24d199 into yansavitski:ai-playground Feb 12, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Open ai flyout #7

Open ai flyout #7

Samiul-TheSoccerFan commented Feb 9, 2024

yansavitski Feb 9, 2024

yansavitski Feb 9, 2024

yansavitski Feb 9, 2024

yansavitski Feb 9, 2024

yansavitski Feb 9, 2024

yansavitski Feb 9, 2024 •

edited

Loading

Samiul-TheSoccerFan Feb 9, 2024

yansavitski Feb 9, 2024

yansavitski Feb 9, 2024

yansavitski Feb 9, 2024

yansavitski commented Feb 9, 2024

yansavitski left a comment

Samiul-TheSoccerFan commented Feb 11, 2024

Open ai flyout #7

Open ai flyout #7

Conversation

Samiul-TheSoccerFan commented Feb 9, 2024

Summary

Checklist

Risk Matrix

For maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yansavitski Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yansavitski commented Feb 9, 2024

yansavitski left a comment

Choose a reason for hiding this comment

Samiul-TheSoccerFan commented Feb 11, 2024

yansavitski Feb 9, 2024 •

edited

Loading