Updated breadcrumb UI to new design #7362

MichaelMauderer · 2023-07-20T22:21:45Z

Pull Request Description

Implements ##7199

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

vitvakatu

The attached screenshot does not match the design:

I see that gaps between elements are different, and not only a gap before the read icon, but also gaps between entry and separator seems a little off to me (I didn't measure though)

vitvakatu · 2023-07-21T13:59:30Z

app/gui/view/component-browser/component-list-panel/breadcrumbs/src/entry.rs

-/// a separator between entries, and an ellipsis. The breadcrumbs implementation selects the needed
-/// representation for each entry in the grid view. For efficiency, text label and icons are
-/// allocated once the entry is created.
+/// An internal structure of [`Entry`]. It has four visual representations: text, an icon (before a


Technically not true, if we look at the variants of State (which was the source of this three representations statement). I'd rather say text has an optional icon.

This comment was out of date and belonged to the first attempt to refactor the breadcrumbs. Fixed now.

vitvakatu · 2023-07-21T14:01:26Z

app/gui/view/component-browser/component-list-panel/breadcrumbs/src/entry.rs

@@ -92,6 +95,10 @@ pub mod ellipsis {
 // === Style ===
 // =============

+/// The width of the icon in a text [`Entry`].
+pub const ICON_WIDTH: f32 = 30.0;


On the attached screenshot, I see the width of the gap between data ▶︎ and icon read to be enormous. I wonder if it is caused by incorrect size or the fact that we don't have separate padding and size settings for the icon. Also, shouldn't it be in the stylesheet?

Fixed the gap and this is now matching the design. This was caused by too large icon size, resulting in space around the icon.

Also, shouldn't it be in the stylesheet?
All the icon sizes here are fixed, so this is following the current implementation.

vitvakatu · 2023-07-21T14:03:53Z

app/gui/view/component-browser/component-list-panel/breadcrumbs/src/entry.rs

+            self.text.set_x(ICON_WIDTH);
+        } else {
+            self.icon.unset_parent();
+            self.icon.set_size((0.0, 0.0));


Is it necessary?

No. Removed.

vitvakatu · 2023-07-21T14:10:00Z

app/gui/view/component-browser/component-list-panel/breadcrumbs/src/lib.rs

+    }
+}
+
+impl Default for Breadcrumb {


It can be derived automatically (recent clippy has a lint for that)

vitvakatu · 2023-07-21T14:12:24Z

app/gui/view/examples/component-list-panel-view/src/lib.rs

+
+        let breadcrumbs = app.new_view::<Breadcrumbs>();
+        breadcrumbs.set_y(400.0);
+        breadcrumbs.frp().set_size(Vector2(500.0, 100.0));
+        breadcrumbs.set_entries_from((
+            vec![
+                Breadcrumb::from("home"),
+                Breadcrumb::from("data"),
+                Breadcrumb::new_with_icon("read", icon::Id::DataInput),
+            ],
+            0,
+        ));
+        breadcrumbs.set_base_layer(&app.display.default_scene.layers.node_searcher);
+
+


Please note that in #7350 breadcrumbs were moved to the documentation view (and to the documentation demo-scene), so there would be a merge conflict in this place.

MichaelMauderer · 2023-07-23T21:39:04Z

The attached screenshot does not match the design: I see that gaps between elements are different, and not only a gap before the read icon, but also gaps between entry and separator seems a little off to me (I didn't measure though)

I've updated the distances to better match the reference (see the new screenshot).

farmaazon

LGTM

farmaazon · 2023-07-25T13:58:47Z

app/gui/view/component-browser/component-list-panel/breadcrumbs/Cargo.toml

@@ -11,3 +11,5 @@ ensogl-derive-theme = { path = "../../../../../../lib/rust/ensogl/app/theme/deri
 ensogl-hardcoded-theme = { path = "../../../../../../lib/rust/ensogl/app/theme/hardcoded" }
 ensogl-text = { path = "../../../../../../lib/rust/ensogl/component/text" }
 ensogl-grid-view = { path = "../../../../../../lib/rust/ensogl/component/grid-view" }
+ide-view-component-list-panel-grid = { path = "../grid" }


Do breadcrumbs still need this dependency?

farmaazon · 2023-07-25T13:59:46Z

app/gui/view/component-browser/component-list-panel/breadcrumbs/src/entry.rs

    Separator,
 }

 #[allow(missing_docs)]
-#[derive(Clone, Copy, CloneRef, Debug, Default, PartialEq)]
+#[derive(Clone, Copy, Debug, Default, PartialEq)]
 enum State {


A private enum, does not need #[allow(missing_docs)]

…ser_breadcrumbs_to_new_design # Conflicts: # CHANGELOG.md

kazcw · 2023-07-27T04:27:52Z

QA: 🟢

…ser_breadcrumbs_to_new_design # Conflicts: # app/gui/view/component-browser/component-list-panel/breadcrumbs/src/lib.rs

Updated breadcrumb UI with optional icons

654de9d

MichaelMauderer self-assigned this Jul 20, 2023

Changelog.

3c26f38

MichaelMauderer marked this pull request as ready for review July 20, 2023 22:30

MichaelMauderer requested review from wdanilo, farmaazon and kazcw as code owners July 20, 2023 22:30

vitvakatu requested changes Jul 21, 2023

View reviewed changes

MichaelMauderer requested a review from vitvakatu July 23, 2023 21:39

MichaelMauderer added 5 commits July 25, 2023 15:54

Correct out of date comment.

b28d191

Remove redundant size change.

27603fe

Derive default implementation.

dfaad50

Update Layout.

8b2dc69

Update Layout.

3ce7a5d

MichaelMauderer requested review from PabloBuchu, indiv0, somebody1234 and Frizi as code owners July 25, 2023 13:55

Revert change to package-lock.json.

e55e84e

somebody1234 removed request for indiv0, PabloBuchu and somebody1234 July 25, 2023 13:58

farmaazon approved these changes Jul 25, 2023

View reviewed changes

MichaelMauderer added 3 commits July 26, 2023 11:22

Remove redundant #[allow(missing_docs)].

1cc825f

Merge branch 'develop' into wip/MichaelMauderer/update_component_brow…

8779dc3

…ser_breadcrumbs_to_new_design # Conflicts: # CHANGELOG.md

Fix layout bug.

535191c

MichaelMauderer added the CI: Ready to merge This PR is eligible for automatic merge label Jul 27, 2023

Merge branch 'develop' into wip/MichaelMauderer/update_component_brow…

20961af

…ser_breadcrumbs_to_new_design # Conflicts: # app/gui/view/component-browser/component-list-panel/breadcrumbs/src/lib.rs

mwu-tow merged commit d272d62 into develop Jul 27, 2023
19 of 21 checks passed

mwu-tow deleted the wip/MichaelMauderer/update_component_browser_breadcrumbs_to_new_design branch July 27, 2023 14:10

kazcw mentioned this pull request Aug 21, 2023

Broken breadcrumbs spacing #7605

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated breadcrumb UI to new design #7362

Updated breadcrumb UI to new design #7362

MichaelMauderer commented Jul 20, 2023 •

edited

Loading

vitvakatu left a comment

vitvakatu Jul 21, 2023

MichaelMauderer Jul 23, 2023

vitvakatu Jul 21, 2023

MichaelMauderer Jul 23, 2023

vitvakatu Jul 21, 2023

MichaelMauderer Jul 21, 2023

vitvakatu Jul 21, 2023

vitvakatu Jul 21, 2023

MichaelMauderer commented Jul 23, 2023

farmaazon left a comment

farmaazon Jul 25, 2023

MichaelMauderer Jul 26, 2023

farmaazon Jul 25, 2023

kazcw commented Jul 27, 2023

Updated breadcrumb UI to new design #7362

Updated breadcrumb UI to new design #7362

Conversation

MichaelMauderer commented Jul 20, 2023 • edited Loading

Pull Request Description

Important Notes

Checklist

vitvakatu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelMauderer commented Jul 23, 2023

farmaazon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazcw commented Jul 27, 2023

QA: 🟢

MichaelMauderer commented Jul 20, 2023 •

edited

Loading