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.

Sign up for GitHub

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

Jump to bottom

[EuiIcon] Add newChat glyph (chat icon for the AI Assistant) #7524

Merged
merged 5 commits into from
Feb 20, 2024
Merged

[EuiIcon] Add newChat glyph (chat icon for the AI Assistant) #7524

merged 5 commits into from
Feb 20, 2024

Conversation

boriskirov
Copy link
Contributor

Summary

Adding a new icon in our EUI Library, it's for the creation of new conversations, new chats for the AI Assistant.

Here is also the Figma link to the branch: LINK

Screenshots:
CleanShot 2024-02-12 at 13 06 24@2x

CleanShot 2024-02-12 at 13 08 54@2x

@boriskirov boriskirov requested a review from a team as a code owner February 12, 2024 12:09
@cee-chen cee-chen changed the title New chat icon for the AI Assistant [EuiIcon] Add newChat glyph (chat icon for the AI Assistant) Feb 12, 2024
MichaelMarcialis
MichaelMarcialis reviewed Feb 12, 2024
View reviewed changes
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, @boriskirov. I left some feedback in Slack for the Figma merge request, but I'll add them here as well for the sake of visibility. All of this feedback is Figma related, but will have some impact on the exported SVG used here:

  • The new icon is contained in a regular frame. Can you change this to a component?

  • The new icon doesn't sit perfectly on the pixel grid. Ideally we would avoid single pixel fills occupying two or more pixels in the grid. Doing so will help prevent the icon from appearing blurry on non-high-pixel-density displays.

  • The contents of the icon's component frame should be a single flatten vector layer titled "Glyph" (to ensure proper inheritance of color styles).

  • The icon's current fill color is black. Would you mind changing this to the default text color or darkest shade?

  • We have a similar/related editorComment icon in the library. Would it make sense to have these two icons both appear and be named similarly so that they can work well if ever in close proximity?

@boriskirov
Copy link
Contributor Author

Thanks @MichaelMarcialis I took all your comments into account and improved the icon, everything is available in the Figma branch and also in this PR:
CleanShot 2024-02-15 at 15 58 24@2x

@cee-chen
Copy link
Member

also in this PR

@boriskirov I'm not seeing any updates in this PR - you may need to pull the changes I pushed up first, re-yarn, and then redo your changes.

@boriskirov
Copy link
Contributor Author

@cee-chen I think it's okay now, I've been a little messy excuse me

@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@CoenWarmer CoenWarmer mentioned this pull request Feb 20, 2024
Merged
@cee-chen cee-chen merged commit b525572 into elastic:main Feb 20, 2024
7 checks passed
@cee-chen cee-chen mentioned this pull request Feb 20, 2024
Merged
jbudz pushed a commit to elastic/kibana that referenced this pull request Feb 23, 2024
@cee-chen
Upgrade EUI to v93.2.0 (#177361)
7481923 
`v93.1.1`⏩ `v93.2.0`

---

- Updated `EuiPageSidebar` and `EuiPageTemplate.Sidebar` with a new
`hasEmbellish` prop (defaults to false)
([#7521](elastic/eui#7521))
- Added `diff` glyph to `EuiIcon`
([#7520](elastic/eui#7520))
- Added `newChat` glyph to `EuiIcon`
([#7524](elastic/eui#7524))

**Bug fixes**

- Fixed `EuiSideNav` not correctly typing the `items` prop as required
([#7521](elastic/eui#7521))
- Fixed the `CSS is not defined` bug in `EuiPageTemplate` when rendering
in some SSR environments, particularly Next.js v13 and up
([#7525](elastic/eui#7525))
- Fixed `EuiDataGrid` component to clean up timer from side effect on
unmount ([#7534](elastic/eui#7534))

**Accessibility**

- Fixed `EuiSideNav` to render a fallback aria-label on mobile toggles
if no heading or mobile title exists
([#7521](elastic/eui#7521))

**CSS-in-JS conversions**

- Converted `EuiSideNav` to Emotion; Removed the following Sass
variables: ([#7521](elastic/eui#7521))
  - `$euiSideNavEmphasizedBackgroundColor`
  - `$euiSideNavRootTextcolor`
  - `$euiSideNavBranchTextcolor`
  - `$euiSideNavSelectedTextcolor`
  - `$euiSideNavDisabledTextcolor`
- Removed the `euiSideNavEmbellish` Sass mixin. Use the new
`EuiPageSidebar` `hasEmbellish` prop instead
([#7521](elastic/eui#7521))
- Added a new memoization/performance optimization utility for CSS-in-JS
styles ([#7529](elastic/eui#7529))
semd pushed a commit to semd/kibana that referenced this pull request Mar 4, 2024
@cee-chen @semd
Upgrade EUI to v93.2.0 (elastic#177361)
192c681 
`v93.1.1`⏩ `v93.2.0`

---

- Updated `EuiPageSidebar` and `EuiPageTemplate.Sidebar` with a new
`hasEmbellish` prop (defaults to false)
([elastic#7521](elastic/eui#7521))
- Added `diff` glyph to `EuiIcon`
([elastic#7520](elastic/eui#7520))
- Added `newChat` glyph to `EuiIcon`
([elastic#7524](elastic/eui#7524))

**Bug fixes**

- Fixed `EuiSideNav` not correctly typing the `items` prop as required
([elastic#7521](elastic/eui#7521))
- Fixed the `CSS is not defined` bug in `EuiPageTemplate` when rendering
in some SSR environments, particularly Next.js v13 and up
([elastic#7525](elastic/eui#7525))
- Fixed `EuiDataGrid` component to clean up timer from side effect on
unmount ([elastic#7534](elastic/eui#7534))

**Accessibility**

- Fixed `EuiSideNav` to render a fallback aria-label on mobile toggles
if no heading or mobile title exists
([elastic#7521](elastic/eui#7521))

**CSS-in-JS conversions**

- Converted `EuiSideNav` to Emotion; Removed the following Sass
variables: ([elastic#7521](elastic/eui#7521))
  - `$euiSideNavEmphasizedBackgroundColor`
  - `$euiSideNavRootTextcolor`
  - `$euiSideNavBranchTextcolor`
  - `$euiSideNavSelectedTextcolor`
  - `$euiSideNavDisabledTextcolor`
- Removed the `euiSideNavEmbellish` Sass mixin. Use the new
`EuiPageSidebar` `hasEmbellish` prop instead
([elastic#7521](elastic/eui#7521))
- Added a new memoization/performance optimization utility for CSS-in-JS
styles ([elastic#7529](elastic/eui#7529))
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
@cee-chen @fkanout
Upgrade EUI to v93.2.0 (elastic#177361)
b761b46 
`v93.1.1`⏩ `v93.2.0`

---

- Updated `EuiPageSidebar` and `EuiPageTemplate.Sidebar` with a new
`hasEmbellish` prop (defaults to false)
([elastic#7521](elastic/eui#7521))
- Added `diff` glyph to `EuiIcon`
([elastic#7520](elastic/eui#7520))
- Added `newChat` glyph to `EuiIcon`
([elastic#7524](elastic/eui#7524))

**Bug fixes**

- Fixed `EuiSideNav` not correctly typing the `items` prop as required
([elastic#7521](elastic/eui#7521))
- Fixed the `CSS is not defined` bug in `EuiPageTemplate` when rendering
in some SSR environments, particularly Next.js v13 and up
([elastic#7525](elastic/eui#7525))
- Fixed `EuiDataGrid` component to clean up timer from side effect on
unmount ([elastic#7534](elastic/eui#7534))

**Accessibility**

- Fixed `EuiSideNav` to render a fallback aria-label on mobile toggles
if no heading or mobile title exists
([elastic#7521](elastic/eui#7521))

**CSS-in-JS conversions**

- Converted `EuiSideNav` to Emotion; Removed the following Sass
variables: ([elastic#7521](elastic/eui#7521))
  - `$euiSideNavEmphasizedBackgroundColor`
  - `$euiSideNavRootTextcolor`
  - `$euiSideNavBranchTextcolor`
  - `$euiSideNavSelectedTextcolor`
  - `$euiSideNavDisabledTextcolor`
- Removed the `euiSideNavEmbellish` Sass mixin. Use the new
`EuiPageSidebar` `hasEmbellish` prop instead
([elastic#7521](elastic/eui#7521))
- Added a new memoization/performance optimization utility for CSS-in-JS
styles ([elastic#7529](elastic/eui#7529))
@boriskirov boriskirov deleted the new-chat-icon branch March 5, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cee-chen cee-chen cee-chen approved these changes

@MichaelMarcialis MichaelMarcialis MichaelMarcialis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@boriskirov @cee-chen @kibanamachine @elasticmachine @MichaelMarcialis