orbtk
orbtk copied to clipboard
redox-os
Improvement for Popup Widget
Hi,
for the application i'm making (and in particular for the 2D engine) i would like to place widgets that float around other widgets or point.
For example if i draw a shape (as a widget) on the 2D engine, i would like to place some text over it with some info, depending on the context. This feature could be also used in a general way, for example: suppose i would like to show a popup when the user hover a button with the mouse. If there is no space to show it, i would like to reposition it on a different side of the button.
So i have taken the Popup Widget as template and made a modified version that implement these features.
I have also modified the Popup example to also show these features.
I have tried to be as less destructive as possible (from API point of view), so it should be a drop in replacement for the current popup.
The standard popup is placed 1 pixel on the bottom of the entity target. By default, the new popup have the same behaviour.
Also, the PopupTarget enum (that replace standard u32 property target) can be derived from u32, so it work for applications and widgets that use current popup (like ComboBox).
Everything can be found on my repository: https://github.com/uniformbuffer/orbtk-popup
I would like to know what do you think, and if you like it i can make a PR.
Thanks for the attention, Have a good day
Sure,
would you like to implement it as a replacement of the current popup or as a separate widget?
Also before push i would like to understand why the popup is decentered relative to the target, otherwise there could be some alignment problem for ComboBox. The formulas seems ok and if i set a point as target, the popup is placed correctly (i have tested this separately, it is not in the example, but i can add it). Maybe i get the target bounds in the wrong moment, so they need to be calculated. bounds are getted during the post_layout_update of the popup (like you suggest), so it should be ok. I need to dig out to find the problem.
Glad to contribute, Have a good day
Edit: In the initial configuration (popup positioned at the bottom of the target), the popup will overlap the target graphically, but from data point of view these should not happen:
Target bounds: Rectangle {
position: Point {
x: 200.0,
y: 250.0,
},
size: Size {
width: 200.0,
height: 200.0,
},
}
Popup bounds: Rectangle {
position: Point {
x: 225.0,
y: 451.0, <----- The +1 is for the distance, that is 1.0 pixel
},
size: Size {
width: 150.0,
height: 150.0,
},
}
I have taken this data from the post_layout_update. The data seems ok, so there are 3 possibilities:
- The widget are moved after the post layout update, so the taken data is "outdated".
- There is some strange behavior in the stretch mechanism that make widgets appear graphically bigger.
- I miss something
Can this be modified to make it look like a tooltip? (like when you mouse over a widget, and a small rectangle shows up with some text)
Can this be modified to make it look like a tooltip? (like when you mouse over a widget, and a small rectangle shows up with some text)
It should be ready for that, it is one of new uses since it can appear over any target. Once the popup is created, it is only needed that the target set the popup "visible" when the mouse hover on it and set "hidden" when not (i have not explored the "hover" mechanism yet, but should work as you expect). You can also make that the target widget itself create the popup and set as target self. You can do that during the "init" phase of your widget, so can retrieve current widget entity and set it as target of the popup.
@uniformbuffer I would like to prefer it as replacement of the current widget. I case of the ComboBox the Popup should be left aligned and not centered to the ComboBox. It also should use the same width of its target. Maybe it would make sense to use the vertical / horizontal alignment property.
Example:
Popup::new().relative_position(RelativePosition::Bottom(1.0)).h_align("stretch")
The Popup will be positioned at the bottom of the target and has the same width as the target and is left aligned:.
Popup::new().relative_position(RelativePosition::Bottom(1.0)).h_align("left")
The Popup will be positioned at the bottom of the target and has the given width and is left aligned.
Popup::new().relative_position(RelativePosition::Left(1.0)).v_align("stretch")
The Popup will be positioned at left of the target and has the same height as the target and is top aligned:.
Popup::new().relative_position(RelativePosition::Bottom(1.0)).h_align("top")
The Popup will be positioned at left of the target and has the given height and is top aligned.
Same for h_align("right"), h_align("center"), v_align("bottom") and v_align("center"). What do you mean?
I found out the source of your problem.
What you need is also the position (global position) of the target let target_position: Point = ctx.get_widget(target.into()).clone("position");. You could check the source code of the Popup widget. Unfortunately that is only calculated after the first run.
Thank you
I case of the ComboBox the Popup should be left aligned and not centered to the ComboBox. It also should use the same width of its target. Maybe it would make sense to use the vertical / horizontal alignment property.
I think using alignment make perfectly sense and give the user the ability to customize more the positioning of the popup. I will implement it.
I found out the source of your problem.
What you need is also the position (global position) of the target
let target_position: Point = ctx.get_widget(target.into()).clone("position");. You could check the source code of the Popup widget. Unfortunately that is only calculated after the first run.
Thanks! I will give it a look.
Hi,
i have changed the way the popup position itself: now use the h_align or v_align (based on the relative_position) to position and measure the popup. I need to test more, but should work.
Also your suggestion to use the position worked perfectly. The only problem is that on first run, the position that i get from the target is (0,0), making the popup position itself wrongly. Changing again the position will get the right position and work as intended.
Finally, after the popup has moved, a strange "trail" is leaved behind. I don't know what is it and what is the cause, i need to investigate.
Just an update, i will keep to work on it.
Have a good day
Hi,
i have fixed the "ghost trail" that have appeared when moving the popup. Basically there was a difference between the bounds of the popup and it's effective shape. I have fixed it applying constraint to the bound, so now have the same shape as the popup itself.
The only bug that remain to fix is the initial wrong positioning due to the initial (0,0) position property of the target. Since the property is correct after the first time the popup move, maybe it a simply ordering problem: could be possible that the position property of the target is calculated after the popup post_layout_update? Maybe i could move all the logic into an appropriate layout (like it was on the original popup). Initially i wrote the logic on the update_post_layout because i was worried of wrong positioning due to the fact that the target need to be positioned before the popup. I have tried to move the logic on update phase and seems to work anyway, so probably my worries were unnecessary.
So i could try to move all the logic into a Layout and see if it fix the problem.
Do you think all of this have sense or the problem could be caused by something else?
Thanks for the attention, Have a good day
PS: bonus question: there are some particular reasons to not just register every widget's property on on_change_filter? Maybe some performance impact?
Hi, position is calculated on rendering. On layout arrange the tree will iterate on reverse order. Therefore you do not have your parents position on this time. If we need the position directly after layout and before rendering we need an extra iteration through the tree. It is possible but has a little impact to the performance.
PS: bonus question: there are some particular reasons to not just register every widget's property on on_change_filter? Maybe some performance impact?
Yes because of the performance impact.
Have a nice day too.
position is calculated on rendering.
So what if i put, at least some, logic into a RenderObject? Now the popup use the Rectangle render object, maybe i can make a custom render object. What do you think?
Yes because of the performance impact.
It's a bit off topic question, but, there is a way to add a filter instead of setting all together, so i can add a filter without touching the ones already setted? This could allow to reduce the performance impact of the filters while leaving some freedom the user. For example, suppose that i want to get notified every time a NumericBox change it's value. Internally the NumericBox could have no interest in such callback, so during construction the NumericBox do not set it. At this point i could add from the outside the callback that i need, without forcing NumericBox enable it by default for everyone or without overwriting already setted callbacks. Maybe a more polished solution could be get an object that represent the added callback, so that can be used to remove it. In this way, only who add the callback have the ability to modify it. This could help also to track the target of the popup: i could make the popup add a callback to the target that notify the popup when it moves. If the target is changed, the callback is then removed from the old target and added to the new one. What do you think?
Have a good day
@uniformbuffer
So what if i put, at least some, logic into a RenderObject? Now the popup use the Rectangle render object, maybe i can make a custom render object. What do you think?
I think for now this should work. It's ok if you want to add something like a PopupRenderObject.
What do you think?
Do you think something like:
NumericBox::new().on_changed("val", |states| println!("Value changed").on_changed("min", |states| println!("Min changed"))
I think that is possible.
I think for now this should work. It's ok if you want to add something like a PopupRenderObject.
Ok, i will try it.
Do you think something like:
NumericBox::new().on_changed("val", |states| println!("Value changed").on_changed("min", |states| println!("Min changed"))
Yes exaclty, something like that. So in this way i could add callbacks without compromising the performance of who does not need them.
Yes exaclty, something like that. So in this way i could add callbacks without compromising the performance of who does not need them.
Ok I think this will be one of my next topics ;-). I like that idea, thank you.
Yes exaclty, something like that. So in this way i could add callbacks without compromising the performance of who does not need them.
Done and it works really great :-)
Hi,
i have made the PopupRenderObject and moved the logic into. It seems to work correctly, the popup is rendered correctly at first try. Unfortunately i have a problem with the ComboBox: it should work correctly, but for some reason the opening mechanism does not work and i don't know why. I have also added the open property like there was in the original popup.
If you agree i would like to make the PR, so you can give a look at the code and tell me what you think. Obviously the PR will be merged only when all the bugs have been fixed.
Have a good day