[Composable template] Create / Edit wizard #70220
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sebelga
added
Feature:Index Management
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
There was a problem hiding this comment.
@sebelga this is looking great!
I'd like to do a second round of reviewing the code/testing, but wanted to leave some initial thoughts.
- Do you think we should differentiate (maybe just via the page title) when the user is creating a legacy template vs. composable template? I also noticed the doc link when creating a legacy template points to composable templates.
- Is the
_metafield optional? If so, should it be marked as so? - In the ingest node pipelines UI, we added a toggle for some of the optional fields. I also followed this pattern with component templates. For composable templates, I'm wondering if this also makes sense for
versionand_meta. WDYT?
Example:
-
Should the review step for composable templates include the additional fields, if defined (e.g.,
_meta,priority,composed_of)? -
Nice job with the DnD functionality! I think it would also be nice if we could somehow explain to the user that the order of the components templates is important.
-
Did you consider using something like
EuiSelectablefor the component templates list? It feels a little weird to me with the select action on the far right. Possibly just me 😄. I'd be interested in feedback from design and others. -
Possible bug: If there's an error when saving the template, and the user goes back a step to resolve it, the error persists.
| @@ -150,6 +150,10 @@ export function useMultiContent<T extends object>({ | |||
| * Validate the multi-content active content(s) in the DOM | |||
| */ | |||
| const validate = useCallback(async () => { | |||
| if (Object.keys(contents.current).length === 0) { | |||
There was a problem hiding this comment.
👍 thanks for fixing this
|
|
||
| function fuzzyMatch(searchValue: string, text: string) { | ||
| const pattern = `.*${searchValue.split('').join('.*')}.*`; | ||
| const re = new RegExp(pattern); |
There was a problem hiding this comment.
nit: might be more readable as const regex
| <EuiFlexItem grow={false}> | ||
| <EuiText>{component.name}</EuiText> | ||
| </EuiFlexItem> | ||
| <EuiFlexItem grow={false} style={{ flexDirection: 'row' }}> |
There was a problem hiding this comment.
Can this inline style be added to component_templates_list_item.scss?
| const emptyPromptBody = ( | ||
| <EuiText color="subdued"> | ||
| <p> | ||
| {text ?? ( |
There was a problem hiding this comment.
When/why would text not be provided?
There was a problem hiding this comment.
I didn't want to mark it as a required prop for the consumer as I defined a default text to be shown. Here we want to have the same copy for the description (below the step title) when there are components, and the description for the empty prompt, so I am explicitly providing the text.
| /> | ||
| )} | ||
| <br /> | ||
| <EuiLink href={docUri} target="_blank"> |
There was a problem hiding this comment.
I know we're not doing this everywhere, but I think it is now preferred to set the external prop for external links.
| @@ -0,0 +1,85 @@ | |||
| /* | |||
There was a problem hiding this comment.
Do you plan on implementing this in a follow-up PR? (I realize part of it depends on the work I'm doing in #69732.)
There was a problem hiding this comment.
Yes this would be part of the enhancement. Not sure it will make it to 7.9 though.
sebelga
commented
Jun 30, 2020
| initialEntries: [`/indices`], | ||
| componentRoutePath: `/:section(indices|templates)`, | ||
| initialEntries: [`/templates`], | ||
| componentRoutePath: `/templates/:templateName?`, |
There was a problem hiding this comment.
@cjcenizal for info this is how you get the memory router to provide the "templateName" to the component.
|
Thanks for the review @alisonelizabeth ! I addressed most of your concerns, can you have another look?
Good point, I've added an
As we haven't done it for the optional fields of the legacy wizard ("version", "merge order"), I prefer to leave it consistent for now. We can revisit this later though.
Great catch. Totally forgot! 😄
I am not sure yet how to indicate it. Happy to hear feedback on that at the demo tomorrow. Did you consider using something like EuiSelectable for the component templates list? It feels a little weird to me with the select action on the far right. Possibly just me 😄. I'd be interested in feedback from design and others. I did not consider the EuiSelectable as I wanted to have a symmetric UI in the left and right columns. This UX is similar to our tables behavior where actions are on the right. This makes me think that I could make the component name clickable to show the flyout and remove the eye icon.
I don't consider this a bug. This is how we currently have it with the legacy wizard. When the user goes back to "index settings" for example, he still sees the error that helps him make any correction. Otherwise, he would have to think "What was that error again?" |
|
@elasticmachine merge upstream |
alisonelizabeth
approved these changes
Jul 2, 2020
There was a problem hiding this comment.
Thanks for making the changes @sebelga! I think this is in good shape to merge and any other UX improvements can be made in follow-up PR.
I did not consider the EuiSelectable as I wanted to have a symmetric UI in the left and right columns. This UX is similar to our tables behavior where actions are on the right. This makes me think that I could make the component name clickable to show the flyout and remove the eye icon.
Let's see what others think at the demo and after playing with it.
👍 Happy to wait and get feedback from others.
I don't consider this a bug. This is how we currently have it with the legacy wizard. When the user goes back to "index settings" for example, he still sees the error that helps him make any correction. Otherwise, he would have to think "What was that error again?"
Good point. I didn't realize this was existing functionality.
Another thing I noticed, but not blocking - I see you refer to "Component templates" as "Components" in the UI. I think it's worth reaching out to the docs team to see if this shorthand is OK.
Also, as discussed privately, the current index template component integration tests are being skipped. I think we should try to get these tests up and running again at some point to make sure we haven't introduced any regressions.
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
|
🎉 This is looking amazing so far, @sebelga! Great work on adding in all of this functionality. I have some suggestions regarding some aspects of the UX which were touched upon during the review. Possible bug?What does this 0 mean in the detail panel? Wasn't sure if this was a bug.
Composition previewI agree with Lee's suggestion to hit the simulate endpoint when fetching the content of an index template detail panel, so we can show the user the resolved ILM policy name in the summary tab, as well as prepopulate a "Composition preview" tab with the results. In terms of where to place a "Preview composition" button within the wizard, I think the bottom bar is still an option if you're open to some hacking. You might be able to resolve the layering conflict with the flyout by adding a custom class to the bottom bar to set a lower z-index than the flyout. This way the flyout will appear on top of the bottom bar. If you'd prefer not to spend those cycles this late in the dev process, then I suggest placing the button near the step's description so it's impossible to miss. We'd repeat this for each section where a preview makes sense (for example we'd exclude it from the first step). I think another reasonable location would be near the back/next buttons, but on the opposite side. I suggest that when the user clicks this button for a step, we only show the part of the composition that's relevant for that step -- for example, if you're on the settings step, we only show the composed settings, but not the aliases or mappings. I think this will help the user focus on the changes they're making in that specific step without getting distracted by less-than-relevant information.
Making MSA clearer in the tableI realize how confusing it is to see the MSA badges when none are selected. I suggest that if none are selected for a row, we just render the word "None". That'll address the confusion, while giving us back space in the table for showing the data stream configuration.
Component template selection UXI have a couple qualms about the current UX for selecting component templates. I'm mostly concerned about the amount of information we're showing the user at one time. If the user is reviewing an index template but isn't interested in adding new component templates, then the list of unselected component templates isn't useful and isn't pertinent information, so it becomes noise. The essence of UX design is defining points in time at which to surface parts of the UI, and I think we can amortize some parts of this UI along the time dimension to address this concern. I'm also worried about the scalability of the component template list when there are many component templates to choose from. I think we can solve this by using a standard EUI table component to list unselected component templates instead of a custom UI. My suggestion would be to leverage the standard table & detail panel pattern we've used elsewhere. It's familiar to users and it doesn't overload them with too much info at one time. When no component templates have been selected, we would just show the standard empty state:
The user will open the selection UI inside a flyout, similar to the UX pattern of configuring a rollup job. I can anticipate a user possibly wanting to drill down into a component template so they can be confident they want to select it, but I think we could add that as a future enhancement if we don't have time for 7.9:
When component templates are selected, they'd show up in the drag-and-drop list you built, though I suggest adding "columns" to help people vertically scan the MSA information:
The user can click the name of the component template to view its details in the flyout:
|
@cjcenizal this is a bug and should be fixed in #69732. |
…e-template-wizard-2
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jul 3, 2020
* master: (199 commits) [Telemetry] Add documentation about Application Usage (elastic#70624) [Ingest Manager] Improve agent unenrollment with unenroll action (elastic#70031) Handle timeouts on creating templates (elastic#70635) [Lens] Add ability to set colors for y-axis series (elastic#70311) [Uptime] Use elastic charts donut (elastic#70364) [Ingest Manager] Update registry URL to point to snapshot registry (elastic#70687) [Composable template] Create / Edit wizard (elastic#70220) [APM] Optimize services overview (elastic#69648) [Ingest Pipelines] Load from json (elastic#70297) [Rum Dashbaord] Rum selected service view (elastic#70579) [Uptime] Prevent duplicate requests on load for index status (elastic#70585) [ML] Changing shared module setup function parameters (elastic#70589) [Ingest Manager] Add ability to sort to agent configs and package configs (elastic#70676) [Alerting] document requirements for developing new action types (elastic#69164) Fixed adding an extra space character on selecting alert variable in action text fields (elastic#70028) [Maps] show vector tile labels on top (elastic#69444) chore(NA): upgrade to lodash@4 (elastic#69868) Add Snapshot Restore README with quick-testing steps. (elastic#70494) [EPM] Use higher priority than default templates (elastic#70640) [Maps] Fix cannot select Solid fill-color when removing fields (elastic#70621) ...
sebelga
added a commit
that referenced
this pull request
Jul 3, 2020
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jul 3, 2020
* master: [Lens] Fitting functions (elastic#69820) [Telemetry] Add documentation about Application Usage (elastic#70624) [Ingest Manager] Improve agent unenrollment with unenroll action (elastic#70031) Handle timeouts on creating templates (elastic#70635) [Lens] Add ability to set colors for y-axis series (elastic#70311) [Uptime] Use elastic charts donut (elastic#70364) [Ingest Manager] Update registry URL to point to snapshot registry (elastic#70687) [Composable template] Create / Edit wizard (elastic#70220) [APM] Optimize services overview (elastic#69648)
|
Thanks for the review @cjcenizal ! There are great ideas that we'd need to discuss further. As we are very late in the cycle and this week no new functionality should be added I will make the least changes on the current UX. We can revisit in Composition previewI will go and put the button on the bottom right of the screen. This is where a user's eyes go automatically. I think it would help a lot.
This is a great idea that I'd like to explore in Making MSA clearer in the tableGreat suggestion, I will do that 👍 |
sebelga
added
release_note:enhancement
and removed
release_note:skip
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/security/doc_level_security_roles·js.security app dls user East should only see EAST docStandard OutStack TraceKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/security/doc_level_security_roles·js.security app dls user East should only see EAST docStandard OutStack TraceKibana Pipeline / kibana-xpack-agent / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_management/data_streams·ts.apis management index management Data streams Get returns an array of all data streamsStandard OutStack Traceand 1 more failures, only showing the first 3. Build metrics@kbn/optimizer bundle module count
History
To update your PR or re-run it, just comment with: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the Creation and Edition UI flows for the composable index templates.
How to test
Start by creating a couple of component templates in Dev tools
PUT /_component_template/base-logs { "version": 1, "template": { "mappings": { "properties": { "title": { "type": "text"} } } } } PUT /_component_template/base-metrics { "version": 1, "template": { "mappings": { "properties": { "myField": { "type": "float"} } }, "aliases": { "someAlias": { "index_routing": "1" } } } }Now navigate to the index management > Index templates (tab)
In our example, filtering only with index settings should return no results as we haven't added any index settings to our components.
The next 3 steps are the same as with legacy index templates. Add any index settings, mappings or aliases as you want.
In the "Review" steo, under the "Request" step, the API endpoint should start with "PUT _index_template/..." (in contrast with legacy index templates whose API is "PUT _template/...")
Click "Save template". The template should save and you should be redirected to the index templates list page.
There should be an "edit" icon in the table that lets you edit the template. Go ahead and edit the template making any modification to the template. To verify that the modification have been saved, edit once again the template and check the changed made were persisted.
Test that legacy index templates work as before
Screenshots
Release note
The index templates tab allows users to manage both their legacy index templates and composable index templates. Users can create, edit, clone, and delete a composable index template.