-
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
[ML] Create watch from new jobs list #21112
[ML] Create watch from new jobs list #21112
Conversation
💚 Build Succeeded |
Pinging @elastic/ml-ui |
|
||
import { toastNotifications } from 'ui/notify'; | ||
import { loadFullJob } from '../utils'; | ||
import { mlCreateWatchService } from 'plugins/ml/jobs/new_job/simple/components/watcher/create_watch_service'; |
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.
just a note that jest tests won't work with that path style because they don't support the automatic public
insertion.
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'll update the paths in these files. However there are a lot of places where this full path is used and will need to be changed.
This can happen in a future PR
|
||
} | ||
} | ||
CreateWatchModal.propTypes = { |
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.
compile
is used as a prop
in the component but not added here to PropTypes
.
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.
good spot. compile
isn't used anymore
}; | ||
} | ||
|
||
export class CreateWatchModal extends Component { |
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.
Should this be CreateWatchFlyout
?
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 with latest changes.
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.
A couple of aria-labels
need changing I think, but otherwise LGTM
Now - <EuiFieldText | ||
value={this.state.interval} | ||
onChange={this.onIntervalChange} | ||
aria-label="Use aria labels when no actual label is in use" |
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.
Did you mean to leave this aria-label
value in like this? Shouldn't it say something like Set to now
?
value={this.state.email} | ||
onChange={this.onEmailChange} | ||
placeholder="email address" | ||
aria-label="Use aria labels when no actual label is in use" |
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.
As above, this aria-label
text looks like a copy and paste from the docs. It should say something like 'email address' I think
💚 Build Succeeded |
* [ML] [WIP] Create watch from new jobs list * removing comments * adding interval calculation * adding checkbox to start datafeed modal * adding proptypes check to SelectSeverity * fixing typo * changes based on review * correcting input labels
* [ML] [WIP] Create watch from new jobs list * removing comments * adding interval calculation * adding checkbox to start datafeed modal * adding proptypes check to SelectSeverity * fixing typo * changes based on review * correcting input labels
Related to #20150 |
Adds the missing create watch option when starting a datafeed which runs in real-time.
New react version of the watch configuration component.
The react version lives along side the original angular version and uses most of the same css.
Flyout has to be used rather than a modal because of this eui issue: elastic/eui#600
Upon creation, a toast notifies the user. The link to edit the new watch has been added as a button to the toast.
If a watch for this job already exists, a warning is shown.