Add ContextMenu to Notification Status Bar item #1211
Conversation
| [@deriving show({with_path: false})] | ||
| type item('data) = { | ||
| label: string, | ||
| // icon: option(IconTheme.IconDefinition.t), |
There was a problem hiding this comment.
IconTheme currently lives in Model. I'm not sure if Core is the right place for it, so for now we'll have to do without.
|
Looks awesome @glennsl !
Sorry about the shadow bug (that would help it 'pop' for sure) - i'll look at adding the 'box shadow' style back in Revery as a temporary workaround. Change looks great! |
glennsl
force-pushed
the
feature/ux/context-menu
branch
from
920531f
to
e97518a
Compare
|
/azp run |
1 similar comment
|
/azp run |
glennsl
changed the title
glennsl
force-pushed
the
feature/ux/context-menu
branch
from
e97518a
to
9f8d779
Compare
glennsl
changed the title
| boxShadow( | ||
| ~xOffset=-5., | ||
| ~yOffset=-5., | ||
| ~blurRadius=25., | ||
| ~spreadRadius=-10., | ||
| ~color=Color.rgba(0., 0., 0., 0.0001), | ||
| ), |
There was a problem hiding this comment.
It works! But it's also kind of broken. The alpha value of the color doesn't seem to be respected, and there seems to be some built-in offset that has to be countered by specifying negative offsets.
I managed to tone it down a bit by adding a negative spread, but it still looks a bit off.
There was a problem hiding this comment.
It works!
Hooray! 🎉
But it's also kind of broken. The alpha value of the color doesn't seem to be respected, and there seems to be some built-in offset that has to be countered by specifying negative offsets.
Ya, unfortunately there are bugs and quirks with the current implementation.
Skia will give us a more robust set of primitives for stuff like this!
There was a problem hiding this comment.
Thanks for the pointer! Looks like fixing it so the alpha value is taken into account shouldn't be too hard, just pass in a vec4 instead of a vec3 and multiply the alpha value with the blur value. But the offset bug seems trickier. I can't se where that comes from. That's also less of an issue I think though, so it can definitely wait for Skia.
|
This should be good to go now. See the latest changes in https://github.com/onivim/oni2/pull/1211/files/3bc5690246786b4622eb9cfc28fe301790537212..9f8d77984c20c29990e34cb27dd05e9c57d31c71#diff-7fdbd38419fb3f5323ee3440ba789a9e |
|
/azp run |
|
Just tried the latest - looks so much nicer with the drop shadow! |
Although perhaps more importantly, this adds a ContextMenu component for general use.