[ML] Adding group selector to jobs management #21780
Conversation
💚 Build Succeeded |
jgowdyelastic
added
review
v7.0.0
:ml
Feature:Anomaly Detection
|
Pinging @elastic/ml-ui |
jgowdyelastic
changed the title
|
|
||
|
|
||
|
|
||
| export const KEY_CODES = { |
There was a problem hiding this comment.
Could you use eui/src/services/key_codes.js from eui instead? It has all the key codes you need I think.
| if (c.jobIds.some(j => jobIds.includes(j))) { | ||
| p[c.id] = { | ||
| partial: (groupCounts[c.id] !== jobIds.length), | ||
| // jobsIds: c.jobIds, |
There was a problem hiding this comment.
Is this commented out line needed?
| closePopover={() => this.closePopover()} | ||
| > | ||
| <div className="group-selector"> | ||
| <EuiPopoverTitle>Apply groups to jobs</EuiPopoverTitle> |
There was a problem hiding this comment.
nit - is it worth labelling as job if only one job is selected?
|
|
||
| import './styles/main.less'; | ||
| import { validateGroupNames } from '../../../validate_job'; | ||
| import { KEY_CODES } from 'plugins/ml/../common/constants/key_codes'; |
There was a problem hiding this comment.
Would need to use a relative path when creating Jest tests, or use the eui keyCodes import instead
💚 Build Succeeded |
| return ( | ||
| <div className="check" /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
AFAIK it's best practice to add return null if a component should conditionally not render at all https://reactjs.org/docs/conditional-rendering.html#preventing-component-from-rendering
There was a problem hiding this comment.
I agree with that practice, however this is not just an empty div, it has a class which supplies necessary styling.
Without it the unselected groups are shifted over to the left:
There was a problem hiding this comment.
You're right, sorry I misread the code and thought there might be cases where nothing would be returned ...
💚 Build Succeeded |
* [ML] [WIP] Adding group selector to jobs management * adding group name validation * removing comment * adding keyboard events * moving new group input to its own component * changes based on review * adding tooltip * adding better error reporting
* [ML] [WIP] Adding group selector to jobs management * adding group name validation * removing comment * adding keyboard events * moving new group input to its own component * changes based on review * adding tooltip * adding better error reporting
Adding a group selection menu to the jobs management page.
When selecting jobs which do not contain the same groups, the partially matched groups are marked with a
-.The user can then click to select to apply to all jobs, or unselect to remove from current jobs.
When adding a new group, the name is validated.
Missing up and down key navigation of groups. This will be added in a follow up accessibility PR.
Missing jest tests. These will be added in a follow up PR.