Adding icons to the context menu #3811
Conversation
maxphilippov
commented
May 8, 2024
maxphilippov
commented
- add icon to ContextMenuItem and render those
- use our ContextMenu for AttachmentMenu
| /> | ||
| ))} | ||
| </> | ||
| <Button |
There was a problem hiding this comment.
use a normal button element here and adjust styling with the attachment-button class.
we want to get rid of blueprintjs and it currently looks bad in dark theme:
(I know this is just a draft, but wanted to give that feedback already anyway)
There was a problem hiding this comment.
Pushed an update for this, there's only paperclip icon styling left (change color and make it a bit smaller).
I made copies of image.svg for icons used before ('document', 'media', 'phone'), should I just copy old svg data there or we need to make new ones?
There was a problem hiding this comment.
just download icons from https://fonts.google.com/icons that look similar / have the same message if there are no matching icons in images/icons yet. images/icons is the folder for the icons.
There was a problem hiding this comment.
I think we should not keep using the blueprint icons unless we want to specify that they have a different license than the google/material-design ones.
|
Nice doing here! |
maxphilippov
force-pushed
the
maxph/3186-context-menu-icons
branch
from
2cdbe5b to
e7c6883
Compare
|
So I took the liberty to change Buttons on a navbar since I had to figure out the way to change color on attachment button and it seemed reasonable to look at all the cases. I'm not 100% happy with my solution on I'm in no hurry to merge this, just commenting on my latest commit here for the future discussion after we ship 1.46 |
maxphilippov
force-pushed
the
maxph/3186-context-menu-icons
branch
from
fa54313 to
61e5f2b
Compare
|
needs to be rebased to current main branch. (also make sure you have made the name change locally: |
maxphilippov
force-pushed
the
maxph/3186-context-menu-icons
branch
2 times, most recently
from
f52db98 to
bd96f3b
Compare
* add icon to ContextMenuItem and render those * use our ContextMenu for AttachmentMenu * replace BlueprintJS components with ours for chat navbar and attachment menu
|
Looks good to me, except for the irregular margins of menu items (which is not related to this PR)
|
maxphilippov
force-pushed
the
maxph/3186-context-menu-icons
branch
from
7b34b67 to
6951807
Compare
I'm up for that, can change this today |
