Skip to content

[ML] Replace View Forecast button image with Single Metric icon - #34563

Merged
peteharverson merged 2 commits into
elastic:masterfrom
peteharverson:ml-view-forecast-button
Apr 5, 2019
Merged

peteharverson merged 2 commits into
elastic:masterfrom
peteharverson:ml-view-forecast-button

Conversation

@peteharverson

@peteharverson peteharverson commented Apr 4, 2019 •

Copy link
Copy Markdown
Contributor

Summary

Replaces the EuiButton with the Font Awesome fa-line-chart icon with an EuiButtonIcon using a stats icon for the 'View forecast' link to the Single Metric Viewer. This makes it consistent with the control used to link to the Single Metric Viewer used for annotations and for the result view links in the Jobs Management page.

Also adds extra information to the aria-label for the button, adding the 'created date' to help identify which forecast the button is linking to, as used by screen readers.

Before:
image

After:
image

Also switches out the popout icon used to open the Single Metric Viewer from anomalies in the Anomaly Explorer to the stats icon for consistency:

image

Checklist

For maintainers

@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/ml-ui

@peteharverson
peteharverson requested review from a team as code owners April 4, 2019 16:52
@@ -2,31 +2,20 @@
.forecasts-table {

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.

Just a reminder. This doesn't look needed. Might want to just look into using the compressed prop on the table.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good spot @snide. I have replaced the two forecast table scss files with JS props on the table. Can you take another look please.

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@alvarezmelissa87 alvarezmelissa87 left a comment

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.

LGTM ⚡️

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@alvarezmelissa87 alvarezmelissa87 left a comment

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.

Latest changes LGTM @peteharverson

@jgowdyelastic jgowdyelastic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@peteharverson
peteharverson merged commit bbfe50f into elastic:master Apr 5, 2019
@peteharverson
peteharverson deleted the ml-view-forecast-button branch April 5, 2019 15:57
peteharverson added a commit to peteharverson/kibana that referenced this pull request Apr 5, 2019
…tic#34563)

* [ML] Replace View Forecast button image with Single Metric icon

* [ML] Remove table scss overrides and use icon in Explorer view
peteharverson added a commit that referenced this pull request Apr 5, 2019
…) (#34639)

* [ML] Replace View Forecast button image with Single Metric icon

* [ML] Remove table scss overrides and use icon in Explorer view

@walterra walterra left a comment

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.

LGTM

@sophiec20 sophiec20 added the Feature:Anomaly Detection ML anomaly detection label Jun 19, 2019
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
…tic#34563)

* [ML] Replace View Forecast button image with Single Metric icon

* [ML] Remove table scss overrides and use icon in Explorer view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants