feat: ui5-tree (new component) #1580

vladitasev · 2020-05-05T13:19:34Z

ui5-tree:

is based on ui5-list, but does not extend it, composes it instead and proxies properties/slots/events to/from it.
uses ui5-tree-item as an abstract (no-renderer) item recursively
the ui5-list internally uses ui5-li-tree - a specific tree item, not intended for the end user, only for internal usage in the list of the tree
has specific visual design: only top-level nodes are of the default list item color, the other ones are of the alternative list color and have no borders. Also all items are not "actionable" in terms of design
has keyboard handling: left/right arrow drills down/up and collapses/expands
has accessibility: role=tree for the tree itself; role=treeitem, ariaExpanded bound to the expanded property, and ariaLevel bound to the level property for tree list items
has compact/cozy parameters for the arrow and toggle box. Indentation is done by padding, for IE there is a limitation: for both cozy and compact the padding is the same. Therefore, there are 2 padding styles, one for IE (the first), and one for other browsers (the second with calc)
The most important event itemToggle can be prevented by the user in order to fetch tree items dynamically, and then the toggling can be done manually with toggle() on the tree node.
Uses a Mutation Observer on itself, the items don't have managedSlots: true so they never know if they change and they never invalidate the tree directly.
The list items and tree items are linked via _ids so that lithtml can insert/delete, rather than overwrite list items, when tree items are modified.

Additionally, fixed some findings in related files:

List.js small doc fixes
ListItem.css invalid host selector :host() instead of just :host
sizes-parameters.css many duplicated vars deleted

closes: #1550

ilhan007

The code look good to me, it would be good to have few tests

ilhan007

There is sth minor that can be implemented later, but just to mention it, the user cant drill up and down the nodes with ARROW-RIGHT|LEFT keys.

ilhan007

also we will need a sample.html for the playground

ilhan007 · 2020-05-11T17:35:05Z

packages/main/src/Tree.js

+ * @alias sap.ui.webcomponents.main.Tree
+ * @extends UI5Element
+ * @tagname ui5-tree
+ * @appenddocs TreeItem


ilhan007 · 2020-05-11T17:35:24Z

packages/main/src/TreeItem.js

+ * @constructor
+ * @author SAP SE
+ * @alias sap.ui.webcomponents.main.TreeItem
+ * @extends UI5Element


ilhan007 · 2020-05-11T17:36:18Z

packages/main/test/samples/Tree.sample.html

@@ -0,0 +1,188 @@
+<header>


To display since there is special marker:

<header class="component-heading"> <h2 class="control-header">Tree</h2> <div class="component-heading-since"> <span></span> </div> </header>

ilhan007 · 2020-05-11T17:39:55Z

packages/main/test/samples/Tree.sample.html

+    <script>
+		var dynamicTree = document.getElementById("treeDynamic");
+		dynamicTree.addEventListener("itemToggle", function(event) {
+			const item = event.detail.item; // get the node that is toggled


There are some "const" in the Tree.html and Tree.sample.html, maybe we should replace them with vars.

packages/main/src/Tree.js

fifoosid · 2020-05-13T07:23:10Z

packages/main/src/Tree.js

+		/**
+		 * Defines the <code>ui5-tree</code> header.
+		 * <br><br>
+		 * <b>Note:</b> When <code>header</code> is set, the


header slot is set..

fifoosid · 2020-05-13T07:31:38Z

packages/main/src/Tree.js

+	}
+
+	onEnterDOM() {
+		this._observer = new MutationObserver(this.onTreeStructureChange.bind(this));


Perhaps we can check if the _observer is available and not create multiple observers if the tree is added and removed from the DOM

Done, moved to constructor

fifoosid · 2020-05-13T07:32:51Z

packages/main/src/Tree.js

+	}
+
+	get list() {
+		return this.shadowRoot.querySelector(`ui5-list`);


this.getDomRef.querySelector()

Done, when I removed the busy indicator, this.getDomRef() is the list (top-level element) but I kept the getter for semantics

fifoosid · 2020-05-13T07:43:33Z

packages/main/src/Tree.js

+	}
+}
+
+const walkTree = (el, level, callback) => {


Have you considered walkTree & buildTree to be static methods in the Tree class?

Yeah, I thought about it, but I prefer them hidden for the moment

vladitasev added 10 commits May 5, 2020 16:16

PoC: ui5-tree

2849531

expand button

2ac2eab

introduce tree list item

c1e3f75

dispatch event to tree

1ce9e1e

tree emits events

f8340f5

modes and events

cd054b5

add selected

d4dee00

icons, header and walk

ee0258a

simplify styles

5a388d5

fix api

912b113

vladitasev changed the title ~~PoC: ui5-tree~~ feat: ui5-tree (new component) May 8, 2020

vladitasev requested review from fifoosid, ilhan007 and MapTo0 May 8, 2020 13:37

minor

1de01dd

ilhan007 requested changes May 11, 2020

View reviewed changes

ilhan007 reviewed May 11, 2020

View reviewed changes

vladitasev added 3 commits May 11, 2020 11:45

keyboard handling

b110423

jsdoc part 1

a14bf9c

jsdoc 2

fa0c97a

ilhan007 reviewed May 11, 2020

View reviewed changes

vladitasev added 2 commits May 11, 2020 15:30

allow for dynamic usage

d058ad3

sample and tests

0990582

ilhan007 reviewed May 11, 2020

View reviewed changes

docs/samples fixes, some tests

b91f247

ilhan007 previously approved these changes May 12, 2020

View reviewed changes

compact cozy

0487420

vladitasev dismissed ilhan007’s stale review via 0487420 May 12, 2020 08:01

vladitasev added the for review label May 12, 2020

vladitasev added 8 commits May 12, 2020 14:19

fix width for IE

17fa025

fix ie kb handling

06591c6

add IE comment

6d614b8

add role tree

5a33482

more acc

0d46ef4

acc tests

4e5ee05

return old code

d9403b7

visual design

76049fa

ilhan007 previously approved these changes May 13, 2020

View reviewed changes

fifoosid suggested changes May 13, 2020

View reviewed changes

remove busy

cbe3866

vladitasev dismissed ilhan007’s stale review via cbe3866 May 13, 2020 09:02

vladitasev added 3 commits May 13, 2020 12:25

Merge remote-tracking branch 'origin/master' into ui5-tree

66e3aad

remove unnecessary CSS after list is now 100% width by default

6db1914

fix failing test after hbs change

93281d2

fifoosid approved these changes May 13, 2020

View reviewed changes

vladitasev merged commit 2dd97cf into master May 13, 2020

vladitasev deleted the ui5-tree branch May 13, 2020 10:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ui5-tree (new component) #1580

feat: ui5-tree (new component) #1580

vladitasev commented May 5, 2020 •

edited

Loading

ilhan007 left a comment

ilhan007 left a comment

ilhan007 left a comment

ilhan007 May 11, 2020

ilhan007 May 11, 2020

ilhan007 May 11, 2020

ilhan007 May 11, 2020

fifoosid May 13, 2020

vladitasev May 13, 2020

fifoosid May 13, 2020

vladitasev May 13, 2020

fifoosid May 13, 2020

vladitasev May 13, 2020

fifoosid May 13, 2020

vladitasev May 13, 2020

feat: ui5-tree (new component) #1580

feat: ui5-tree (new component) #1580

Conversation

vladitasev commented May 5, 2020 • edited Loading

ilhan007 left a comment

Choose a reason for hiding this comment

ilhan007 left a comment

Choose a reason for hiding this comment

ilhan007 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

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

vladitasev commented May 5, 2020 •

edited

Loading