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.

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 tooltips to the action bar #6035

Merged
merged 5 commits into from
Mar 27, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
shortened labels for entries with long module paths. When an option is
selected from the dropdown, the necessary module imports are inserted,
eliminating the need for fully qualified names.
- [Icon buttons now have tooltips.][6035]
Copy link
Member

Choose a reason for hiding this comment

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

Please write a better description - it is user-facing changelog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any suggestions? Please be constructive when providing PR comments.

Copy link
Member

Choose a reason for hiding this comment

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

I was sure it was constructive enough. User-facing changelog should describe what is the change, why it was made and what benefits it gives users.

Copy link
Contributor

Choose a reason for hiding this comment

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

I played with GPT for a minute:

[Added tooltips to icon buttons][6035] for improved usability. Users can now quickly understand each button's function.


#### EnsoGL (rendering engine)

Expand Down Expand Up @@ -176,12 +177,14 @@
performed in a given frame][5895]. In particular, you can now inspect names of
all symbols rendered in a given frame. You can also pause the performance
monitor and inspect results recorded in the past.
- [ToggleButtons can now have tooltips][6035].

[3857]: https://github.com/enso-org/enso/pull/3857
[3985]: https://github.com/enso-org/enso/pull/3985
[4047]: https://github.com/enso-org/enso/pull/4047
[4003]: https://github.com/enso-org/enso/pull/4003
[5895]: https://github.com/enso-org/enso/pull/5895
[6035]: https://github.com/enso-org/enso/pull/6035

#### Enso Standard Library

Expand Down
68 changes: 58 additions & 10 deletions app/gui/view/graph-editor/src/component/node/action_bar.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
//! Definition of the `ActionBar` component for the `visualization::Container`.



pub mod icon;

use crate::prelude::*;
use ensogl::display::shape::*;

use enso_config::ARGS;
use enso_frp as frp;
use ensogl::application::tooltip;
use ensogl::application::Application;
use ensogl::display;
use ensogl::display::shape::*;
use ensogl_component::toggle_button;
use ensogl_component::toggle_button::ColorableShape;
use ensogl_component::toggle_button::ToggleButton;
use ensogl_hardcoded_theme as theme;


// ==============
// === Export ===
// ==============

pub mod icon;



// ==================
// === Constants ===
Expand All @@ -27,6 +31,9 @@ const BUTTON_OFFSET: f32 = 0.5;
/// Grow the hover area in x direction by this amount. Used to close the gap between action
/// icons and node.
const HOVER_EXTENSION_X: f32 = 15.0;
const FREEZE_TOOLTIP_LABEL: &str = "Freeze";
const SKIP_TOOLTIP_LABEL: &str = "Skip";
const VISIBILITY_TOOLTIP_LABEL: &str = "Show preview";


// ===============
Expand Down Expand Up @@ -89,11 +96,11 @@ struct Icons {
}

impl Icons {
fn new() -> Self {
fn new(app: &Application) -> Self {
let display_object = display::object::Instance::new();
let freeze = ToggleButton::new();
let visibility = ToggleButton::new();
let skip = ToggleButton::new();
let freeze = labeled_button(app, FREEZE_TOOLTIP_LABEL);
let visibility = labeled_button(app, VISIBILITY_TOOLTIP_LABEL);
let skip = labeled_button(app, SKIP_TOOLTIP_LABEL);
display_object.add_child(&visibility);
if ARGS.groups.feature_preview.options.skip_and_freeze.value {
display_object.add_child(&freeze);
Expand All @@ -115,6 +122,11 @@ impl display::Object for Icons {
}
}

fn labeled_button<Icon: ColorableShape>(app: &Application, label: &str) -> ToggleButton<Icon> {
let tooltip_style = tooltip::Style::set_label(label.to_owned());
ToggleButton::new(app, tooltip_style)
}



// ========================
Expand All @@ -136,7 +148,7 @@ impl Model {
let scene = &app.display.default_scene;
let display_object = display::object::Instance::new();
let hover_area = hover_area::View::new();
let icons = Icons::new();
let icons = Icons::new(app);
let shapes = compound::events::MouseEvents::default();
let size = default();
let styles = StyleWatch::new(&scene.style_sheet);
Expand Down Expand Up @@ -324,3 +336,39 @@ impl display::Object for ActionBar {
self.model.display_object()
}
}



// ============
// === Test ===
// ============

#[cfg(test)]
mod test {
Procrat marked this conversation as resolved.
Show resolved Hide resolved
use super::*;

#[test]
fn test_tooltips() {
let app = Application::new("root");
let action_bar = ActionBar::new(&app);
let visibility_icon = &action_bar.model.icons.visibility;

// By default, the tooltip shouldn't be shown
assert_eq!(app.frp.tooltip.value().content(), None);

// Move the mouse over the visibility button
visibility_icon.view().events.mouse_over.emit(());

// We expect the button to be hovered by the mouse
assert!(visibility_icon.frp.is_hovered.value());

// We expect the tooltip to be shown now
assert_eq!(app.frp.tooltip.value().content(), Some("Show preview"));

// Move the mouse away again
visibility_icon.view().events.mouse_out.emit(());

// We expect the tooltip to be gone
assert_eq!(app.frp.tooltip.value().content(), None);
}
}
5 changes: 4 additions & 1 deletion app/gui/view/graph-editor/src/component/profiling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use ensogl::display::shape::*;
use crate::view;

use enso_frp as frp;
use ensogl::application::tooltip;
use ensogl::application::Application;
use ensogl::data::color;
use ensogl::display;
Expand Down Expand Up @@ -148,7 +149,9 @@ impl Button {
let frp = Frp::new();
let network = &frp.network;

let button = ToggleButton::<icon::Shape>::new();
let tooltip_style = tooltip::Style::set_label("Profile".to_owned())
.with_placement(tooltip::Placement::Left);
let button = ToggleButton::<icon::Shape>::new(app, tooltip_style);
scene.layers.panel.add(&button);
button.set_visibility(true);
button.frp.set_size(Vector2(32.0, 32.0));
Expand Down
45 changes: 31 additions & 14 deletions lib/rust/ensogl/component/toggle-button/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
use ensogl_core::prelude::*;

use enso_frp as frp;
use ensogl_core::application::tooltip;
use ensogl_core::application::Application;
use ensogl_core::data::color;
use ensogl_core::display;
use ensogl_core::display::shape::system::Shape;
Expand Down Expand Up @@ -202,27 +204,30 @@ pub struct ToggleButton<S: Shape> {
model: Rc<Model<S>>,
}

impl<Shape: ColorableShape + 'static> Default for ToggleButton<Shape> {
fn default() -> Self {
let frp = Frp::new();
let model = Rc::new(Model::<Shape>::new());
Self { frp, model }.init_frp()
}
}

impl<Shape: ColorableShape + 'static> ToggleButton<Shape> {
/// Constructor.
pub fn new() -> Self {
default()
pub fn new(app: &Application, tooltip_style: tooltip::Style) -> Self {
let frp = Frp::new();
let model = Rc::new(Model::<Shape>::new());
Self { frp, model }.init_frp(app, tooltip_style)
}

fn init_frp(self) -> Self {
fn init_frp(self, app: &Application, tooltip_style: tooltip::Style) -> Self {
let network = &self.frp.network;
let frp = &self.frp;
let model = &self.model;
let color = color::Animation::new(network);
let icon = &model.icon.events;

// Explicitly define the tooltip placement if none was set. This ensures that this tooltip
// is always correctly placed even when other components use tooltips as well. Otherwise,
// the explicit placement setting of other tooltips would be used, since other tooltips use
// the same application-level FRP node for setting the style.
let tooltip_style = {
let placement = tooltip_style.placement().unwrap_or_default();
tooltip_style.with_placement(placement)
};

frp::extend! { network

// === Input Processing ===
Expand All @@ -241,16 +246,16 @@ impl<Shape: ColorableShape + 'static> ToggleButton<Shape> {

frp.source.mouse_over <+ icon.mouse_over;
frp.source.mouse_out <+ icon.mouse_out;
frp.source.is_hovered <+ bool(&icon.mouse_out, &icon.mouse_over);
frp.source.is_pressed <+ bool(&icon.mouse_up_primary, &icon.mouse_down_primary);


// === Color ===

invisible <- frp.set_visibility.on_false().constant(0.0);
color.target_alpha <+ invisible;

frp.source.visible <+ frp.set_visibility;
frp.source.is_hovered <+ bool(&icon.mouse_out,&icon.mouse_over);
frp.source.is_pressed <+ bool(&icon.mouse_up_primary,&icon.mouse_down_primary);
frp.source.visible <+ frp.set_visibility;

button_state <- all_with4(&frp.visible,&frp.state,&frp.is_hovered,&frp.is_pressed,
|a,b,c,d| ButtonState::new(*a,*b,*c,*d));
Expand All @@ -260,6 +265,18 @@ impl<Shape: ColorableShape + 'static> ToggleButton<Shape> {

color.target <+ color_target;
eval color.value ((color) model.icon.set_color(color.into()));


// === Tooltip ===

tooltip <- frp.is_hovered.map(move |is_hovered| {
if *is_hovered {
tooltip_style.clone()
} else {
tooltip::Style::unset_label()
}
});
app.frp.set_tooltip <+ tooltip;
}

frp.set_state.emit(false);
Expand Down