-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[RAM] Autocomplete #158454
[RAM] Autocomplete #158454
Conversation
…thub.com:guskovaue/kibana into RAM-158244-autocomplete-component-for-action-msg
…-ref HEAD~1..HEAD --fix'
…thub.com:guskovaue/kibana into RAM-158244-autocomplete-component-for-action-msg
…thub.com:guskovaue/kibana into RAM-158244-autocomplete-component-for-action-msg
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 tested it locally, works as expected. I left a few comments regarding the experiment flag, as that is currently not working as expected. Once that is done then I think this is ready to merge.
} | ||
}, [editAction, index, inputTargetValue, isListOpen, paramsProperty])} | ||
onClick={useCallback(() => closeList(), [closeList])} | ||
onScroll={useCallback(() => { |
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.
might be better to have the on scroll listen on the window, since I noticed if i scrolled the outer div, the list doesn't close
useEffect(() => {
window.addEventListener('scroll', closeList, { passive: true, capture: true });
return () => {
window.removeEventListener('scroll', closeList. { capture: true });
};
}, [closeList]);
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've try to fix it fast. Did not work, because when our main logic will be broken and autocomplete will work with bugs.
I've created separate task for that: #165204.
For this PR think that it's fine to keep it like this, because we have similar behaviour in different places in rule form.
Are you ok with that?
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 were able to figure it out check the last commit
x-pack/plugins/stack_connectors/public/components/text_area_with_autocomplete.tsx
Outdated
Show resolved
Hide resolved
* 2.0. | ||
*/ | ||
|
||
import { schema, TypeOf } from '@kbn/config-schema'; |
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.
So I tried this again and noticed 2 issues:
- We need to tell the
stack_connectors
server to expose the enabled experiments to the frontend. To do this, inx-pack/plugins/stack_connectors/server/index.ts
, we need to add
export const config: PluginConfigDescriptor<ConfigSchema> = {
exposeToBrowser: {
enableExperimental: true,
},
schema: configSchema,
};
to the exported config
- So experiment flags are a toggle (either we set them or remove them). So this means
isMustacheAutocompleteon
should default to false (assuming we want autocomplete to be the default state, as we can only add the experiment flag to kibana.yml, not somehow take it away). This means that the experiment should actually beisMustacheAutocompleteOff
, which defaults to false. Then if we want to turn off the new autocomplete we can add the experiment flag (which then turns it off).
So in line 15 of x-pack/plugins/stack_connectors/common/experimental_features.ts
, this needs to default to false
, you will notice a type error in line 39, which is expected, we have an issue to fix this so in the meantime add the // @ts-expect-error ts upgrade v4.7.4
to the line above it.
Then where ever you were using the flag to determine which component to render, you will need to swap the logic around.
…-ref HEAD~1..HEAD --fix'
…thub.com:guskovaue/kibana into RAM-158244-autocomplete-component-for-action-msg
@JiaweiWu Are these fixes what you meant for feature flag? fix experimental flag |
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.
Change in test/plugin_functional/test_suites/core_plugins/rendering.ts LGTM
…complete-component-for-action-msg
propertyPath: string; | ||
}) => { | ||
if (!actionVariablesList) return []; | ||
const allSuggestions: 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.
nit use Set to avoid check includes
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.
LGTM!
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @guskovaue |
## Summary Solves this issue: elastic#161763 This PR introduces autocomplete for mustache variables for email connector(next PR will add it to all connectors) under the feature flag. We decided keep old solution with button with all searchable options as well. How to test: Create an email connector in kibana.yml: xpack.actions.preconfigured: maildev: name: 'email: maildev' actionTypeId: '.email' config: from: 'guskova@example.com' host: 'localhost' port: '1025' How it should work: You start writing in Message window {{ and mustache variable name. And you should see autocomplete popup with all possible options to choose. When you click somewhere else, popup should disappeared. https://github.com/elastic/kibana/assets/26089545/061016a6-b8ca-497b-9bed-b8b012d31a95 e options to choose. When you click somewhere else, popup should disappeared. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Xavier Mouligneau <xavier.mouligneau@elastic.co>
Summary
Solves this issue: #161763
This PR introduces autocomplete for mustache variables for email connector(next PR will add it to all connectors) under the feature flag.
We decided keep old solution with button with all searchable options as well.
How to test:
Create an email connector in kibana.yml:
xpack.actions.preconfigured:
maildev:
name: 'email: maildev'
actionTypeId: '.email'
config:
from: 'guskova@example.com'
host: 'localhost'
port: '1025'
How it should work:
You start writing in Message window {{ and mustache variable name. And you should see autocomplete popup with all possible options to choose. When you click somewhere else, popup should disappeared.
Screen.Recording.2023-07-12.at.16.45.51.mov
e options to choose. When you click somewhere else, popup should disappeared.