Skip to content

Comments

[OneChat] Simple Chat UI#222816

Merged
zacharyparikh merged 40 commits intoelastic:mainfrom
zacharyparikh:onechat/simple-ui
Jun 27, 2025
Merged

[OneChat] Simple Chat UI#222816
zacharyparikh merged 40 commits intoelastic:mainfrom
zacharyparikh:onechat/simple-ui

Conversation

@zacharyparikh
Copy link
Contributor

@zacharyparikh zacharyparikh commented Jun 5, 2025

Summary

Simple.Chat.UI.mov

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • 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
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
  • Flaky Test Runner was used on any tests changed
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

@zacharyparikh zacharyparikh force-pushed the onechat/simple-ui branch 4 times, most recently from 3a003af to 07cef87 Compare June 10, 2025 12:58
@zacharyparikh zacharyparikh force-pushed the onechat/simple-ui branch 3 times, most recently from e62ad4a to ee3b3f8 Compare June 18, 2025 13:05
@pgayvallet
Copy link
Contributor

/ci

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Comment on lines +31 to +40
const setConversation = (updater: (conversation?: Conversation) => Conversation) => {
queryClient.setQueryData<Conversation>(queryKey, updater);
};
const setCurrentRound = (updater: (conversationRound: ConversationRound) => void) => {
setConversation(
produce((draft) => {
const round = draft?.rounds?.at(-1);
if (round) {
updater(round);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't expecting that kind of approach, but I believe it makes sense in the react-query way.

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #37 / InfraOps App Metrics UI Node Details #Asset Type: host without metrics Overview Tab "before all" hook for "cpuUsage tile should be shown"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
onechat 33 103 +70

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
onechat 0.0B 30.4KB +30.4KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
onechat 6.7KB 10.3KB +3.7KB
serverlessSearch 26.5KB 26.6KB +22.0B
total +3.7KB
Unknown metric groups

async chunk count

id before after diff
onechat 0 1 +1

miscellaneous assets size

id before after diff
onechat 0.0B 152.3KB +152.3KB

History

@zacharyparikh zacharyparikh force-pushed the onechat/simple-ui branch 2 times, most recently from 69d2d10 to 71f1681 Compare June 26, 2025 16:29
@gsoldevila gsoldevila removed request for a team June 27, 2025 07:13
To enable the Chat UI located at `/app/chat/`, add the following to your Kibana config:

```yaml
uiSettings.overrides:
Copy link
Member

Choose a reason for hiding this comment

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

thanks for starting 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.

No problem, it was Oren's suggestion


const conversationPanelClass = css`
min-height: 100%;
max-width: 850px;
Copy link
Member

@joemcelroy joemcelroy Jun 27, 2025

Choose a reason for hiding this comment

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

we try not to hardcode widths here. You should be able to reach the euiTheme via hook and get the spacing values.

max-width: calc(${euiTheme.size.xl} * 10);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Styling will likely be changing anyways in follow up

`;

const inputFlexItemClass = css`
max-width: 900px;
Copy link
Member

Choose a reason for hiding this comment

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

same here with hardcoded width

</div>
) : (
<EuiText size="s" color="subdued">
No result available
Copy link
Member

@joemcelroy joemcelroy Jun 27, 2025

Choose a reason for hiding this comment

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

missing localisation but we can ignore here as this is going to have changes.

Copy link
Member

@joemcelroy joemcelroy left a comment

Choose a reason for hiding this comment

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

Looks great! Feel free to merge now without changes and address them in another PR, for the sake of a passing build :)

@zacharyparikh zacharyparikh merged commit 3ef270c into elastic:main Jun 27, 2025
10 checks passed
delanni added a commit that referenced this pull request Jun 27, 2025
## Summary
Fixing a type error coming from a potential race-merge situation. 
#222816 +
#224801
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v9.1.0 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants