Skip to content
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

[EuiIcon] Machine Learning icons for panels #7873

Merged
merged 8 commits into from
Jul 17, 2024
Merged

Conversation

joana-cps
Copy link
Contributor

@joana-cps joana-cps commented Jul 4, 2024

Summary

Set of 7 new icons for Machine Learning panels.

Machine Learning panels, available in Dashboards, didn't have a designated icon. With the upcoming change in the add panel flow in Dashboards it became mandatory for consistency with the other panels.

Screenshot 2024-07-04 at 15 29 16

ml-icon-set-in-context

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

Copy link
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

These new icons look great, @joana-cps! I left a few small comments for your review (and one question for the EUI team). Once those are addressed, I think this will be good to go.

Other than that, in case you haven't already, do feel free to update the Figma EUI library with these new icons so all designers have the ability to use them. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

@elastic/eui-team: Do ya'll have a preference on how we should be naming our SVGs? Should we use underscores or camel casing? Currently we use both. I've always tended to use camel casing because it made things easier in the icon map file. Then again, excluding SVGs, it looks like all other files use underscores. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

We should keep the snake case (underscore) file naming convention whenever possible. The final icon key in icon_map.ts should use camel case and map to the snake-cased file name, e.g., arrowDown: 'arrow_down'

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, @tkajtoch. I'll make sure to do that going forward. Would it be worth opening a separate issue to update all the SVG file names and mappings to reflect this for consistency?

packages/eui/src/components/icon/svgs/field_statistics.svg Outdated Show resolved Hide resolved
packages/eui/src/components/icon/svgs/anomaly_chart.svg Outdated Show resolved Hide resolved
packages/eui/src/components/icon/svgs/field_statistics.svg Outdated Show resolved Hide resolved
joana-cps and others added 5 commits July 9, 2024 09:32
Co-authored-by: Michael Marcialis <michael.l.marcialis@gmail.com>
Co-authored-by: Michael Marcialis <michael.l.marcialis@gmail.com>
Co-authored-by: Michael Marcialis <michael.l.marcialis@gmail.com>
Co-authored-by: Michael Marcialis <michael.l.marcialis@gmail.com>
Co-authored-by: Michael Marcialis <michael.l.marcialis@gmail.com>
@joana-cps
Copy link
Contributor Author

These new icons look great, @joana-cps! I left a few small comments for your review (and one question for the EUI team). Once those are addressed, I think this will be good to go.

Thanks @MichaelMarcialis, all comments are reviewed on my side now.

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Copy link
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes, @joana-cps. LGTM!

@joana-cps joana-cps closed this Jul 17, 2024
@joana-cps joana-cps reopened this Jul 17, 2024
@cee-chen cee-chen merged commit 8e9df72 into elastic:main Jul 17, 2024
7 checks passed
@joana-cps joana-cps deleted the ml-icons branch July 17, 2024 17:39
@cee-chen cee-chen mentioned this pull request Jul 24, 2024
4 tasks
cee-chen pushed a commit to elastic/kibana that referenced this pull request Jul 25, 2024
`v95.3.0` ⏩ `v95.4.0`

_[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_

---

## [`v95.4.0`](https://github.com/elastic/eui/releases/v95.4.0)

- Added `anomalyChart`, `anomalySwimLane`, `changePointDetection`,
`fieldStatistics`, `logPatternAnalysis`, `logRateAnalysis` and
`singleMetricViewer` glyph to `EuiIcon`
([#7873](elastic/eui#7873))

**Bug fixes**

- Fixed overlapping content in `EuiBasicTable` for expanded and
selectable table rows
([#7895](elastic/eui#7895))
- Fixed the alignment of `EuiBasicTable` mobile actions
([#7895](elastic/eui#7895))

**Accessibility**

- Improved `EuiStat`'s screen reader accessibility
([#7864](elastic/eui#7864))

---

## Additional Changes

- reverts temporary fix for overlapping content in nested tables done in
PR [#188374](#188374)

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants