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

Add ContextMenu to Notification Status Bar item #1211

Merged
merged 15 commits into from
Jan 22, 2020
Merged

Add ContextMenu to Notification Status Bar item #1211

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a3b2218
add Components project
glennsl Jan 9, 2020
62ad7c3
add ContextMenu
glennsl Jan 9, 2020
d10d360
use functor to generate identity instead of Identity component
glennsl Jan 16, 2020
d425b63
wire up context menu
glennsl Jan 16, 2020
cce3f18
add context menu to notification status abr item
glennsl Jan 16, 2020
ed0a405
fix warning
glennsl Jan 16, 2020
4484b20
add menu item focus
glennsl Jan 16, 2020
b785a17
extract placement from ContextMenu.t
glennsl Jan 16, 2020
37637ed
ContextMenu -> ContextMenuUpdated
glennsl Jan 18, 2020
3bc5690
post-rebase fix
glennsl Jan 18, 2020
b59409d
originX/Y -> orientation
glennsl Jan 19, 2020
460eb7a
add offsetX/Y
glennsl Jan 19, 2020
8735d1c
adjust origin for orientation
glennsl Jan 19, 2020
77f00ba
add box shadow
glennsl Jan 21, 2020
9f8d779
use default mouse cursor on overlay
glennsl Jan 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 27 additions & 23 deletions src/Components/ContextMenu.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ type item('data) = {
type placement = {
glennsl marked this conversation as resolved.
Show resolved Hide resolved
x: int,
y: int,
originX: [ | `Left | `Middle | `Right],
originY: [ | `Top | `Middle | `Bottom],
orientation: ([ | `Top | `Middle | `Bottom], [ | `Left | `Middle | `Right]),
glennsl marked this conversation as resolved.
Show resolved Hide resolved
};

type t('data) = {
Expand Down Expand Up @@ -142,14 +141,21 @@ module Menu = {
backgroundColor(theme.menuBackground),
color(theme.menuForeground),
width(Constants.menuWidth),
boxShadow(
~xOffset=-5.,
~yOffset=-5.,
~blurRadius=25.,
~spreadRadius=-10.,
~color=Color.rgba(0., 0., 0., 0.0001),
),
Comment on lines +144 to +150
Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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!

Copy link
Member Author

@glennsl glennsl Jan 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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.

];
};

let component = React.Expert.component("Menu");
let make = (~items, ~placement, ~theme, ~font, ~onItemSelect, ()) =>
component(hooks => {
let ((maybeRef, setRef), hooks) = Hooks.state(None, hooks);
let {x, y, originX, originY} = placement;
let {x, y, orientation: (orientY, orientX)} = placement;

let height =
switch (maybeRef) {
Expand All @@ -159,30 +165,20 @@ module Menu = {
let width = Constants.menuWidth;

let x =
switch (originX) {
switch (orientX) {
| `Left => x
| `Middle => x - width / 2
| `Right => x - width
};

let y =
switch (originY) {
switch (orientY) {
| `Top => y - height
| `Middle => y - height / 2
| `Bottom => y
};

(
// TODO: BoxShadow apparently blocks mouse events. Figure out why before adding back
// <BoxShadow
// boxShadow={Style.BoxShadow.make(
// ~xOffset=-11.,
// ~yOffset=-11.,
// ~blurRadius=25.,
// ~spreadRadius=0.,
// ~color=Color.rgba(0., 0., 0., 0.2),
// (),
// )}>
<View
style={Styles.container(~x, ~y, ~theme)}
ref={node => setRef(_ => Some(node))}>
Expand All @@ -193,7 +189,6 @@ module Menu = {
})
|> React.listToElement}
</View>,
// </BoxShadow>,
hooks,
);
});
Expand All @@ -212,6 +207,7 @@ module Overlay = {
left(0),
right(0),
pointerEvents(`Allow),
cursor(MouseCursors.arrow),
];
};

Expand All @@ -235,8 +231,9 @@ module Make = (()) => {
let make =
(
~model as maybeModel,
~originX=`Left,
~originY=`Bottom,
~orientation=(`Bottom, `Left),
~offsetX=0,
~offsetY=0,
~onUpdate,
(),
) =>
Expand All @@ -246,14 +243,21 @@ module Make = (()) => {
switch (maybeModel, maybeRef) {
| (Some(model), Some(node)) =>
if (model.id == id) {
let (x, y, _, _) =
let (x, y, width, _) =
Math.BoundingBox2d.getBounds(node#getBoundingBox());

let x =
switch (orientation) {
| (_, `Left) => x
| (_, `Middle) => x -. width /. 2.
| (_, `Right) => x -. width
};

let placement =
Some({
x: int_of_float(x),
y: int_of_float(y),
originX,
originY,
x: int_of_float(x) + offsetX,
y: int_of_float(y) + offsetY,
orientation,
});

if (model.placement != placement) {
Expand Down
9 changes: 7 additions & 2 deletions src/Components/ContextMenu.rei
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,13 @@ module Make:
let make:
(
~model: option(t('data)),
~originX: [ | `Left | `Middle | `Right]=?,
~originY: [ | `Top | `Middle | `Bottom]=?,
~orientation: (
[ | `Top | `Middle | `Bottom],
[ | `Left | `Middle | `Right],
)
=?,
~offsetX: int=?,
~offsetY: int=?,
~onUpdate: t('data) => unit,
unit
) =>
Expand Down
3 changes: 2 additions & 1 deletion src/UI/StatusBar.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ let notificationCount =

<item onClick onRightClick>
<Notifications.ContextMenu.Anchor
originY=`Top
orientation=(`Top, `Left)
offsetX=(-10) // correct for item padding
model=contextMenu
onUpdate=onContextMenuUpdate
/>
Expand Down