Skip to content

[WorkChat] Change index selector to a ComboBox for the "Index Source" integration#216998

Merged
artem-shelkovnikov merged 14 commits into
mainfrom
artem/add-index-search-combobox
Apr 14, 2025
Merged

[WorkChat] Change index selector to a ComboBox for the "Index Source" integration#216998
artem-shelkovnikov merged 14 commits into
mainfrom
artem/add-index-search-combobox

Conversation

@artem-shelkovnikov
Copy link
Copy Markdown
Member

@artem-shelkovnikov artem-shelkovnikov commented Apr 3, 2025

Closes https://github.com/elastic/search-team/issues/9656

Summary

This PR adds changes the input that allows user enter the index when configuring a WorkChat integration with "Index Source".

The video is better than a thousand words:

Before:

Screen.Recording.2025-04-04.at.18.23.52.mov

After:

Screen.Recording.2025-04-11.at.18.26.07.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

@artem-shelkovnikov artem-shelkovnikov changed the title WIP [WorkChat] Change index selector to a ComboBox for the Choose your Index flow Apr 4, 2025
@artem-shelkovnikov artem-shelkovnikov changed the title [WorkChat] Change index selector to a ComboBox for the Choose your Index flow [WorkChat] Change index selector to a ComboBox for the "Index Source" integration Apr 4, 2025
@artem-shelkovnikov artem-shelkovnikov marked this pull request as ready for review April 4, 2025 16:42
@artem-shelkovnikov artem-shelkovnikov requested a review from a team as a code owner April 4, 2025 16:42
Copy link
Copy Markdown
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.

Functionally, it's looking okay!

Added comments / suggestions on improvements (note: those can be ignored if this is meant to be something temporary. I have no idea how much we're thinking of fully trashing the current UI for integration configurations)

Comment thread x-pack/solutions/chat/plugins/wci-index-source/server/routes/configuration.ts Outdated
Copy link
Copy Markdown
Contributor

@jedrazb jedrazb left a comment

Choose a reason for hiding this comment

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

LGTM, some nits

return response.indexNames;
},
onError: (err: any) => {
notifications.toasts.addError(err, { title: 'Error fetching indices' });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we are surfacing the error in a toast, we should wrap the error string with the i18n logic

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've copied it over from another WorkChat component - none of components are using i18n, so it should be fine to not do it here? Or should I change this everywhere in the code within the PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think it should be fine to not worry about i18n at this stage

@artem-shelkovnikov artem-shelkovnikov added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Apr 9, 2025
@artem-shelkovnikov artem-shelkovnikov enabled auto-merge (squash) April 14, 2025 14:37
@artem-shelkovnikov artem-shelkovnikov merged commit 5ee5b35 into main Apr 14, 2025
10 checks passed
@artem-shelkovnikov artem-shelkovnikov deleted the artem/add-index-search-combobox branch April 14, 2025 16:15
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
wciIndexSource 13 16 +3

Page load bundle

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

id before after diff
wciIndexSource 20.3KB 21.9KB +1.6KB

History

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants