Skip to content

Conversation

@dgliwka
Copy link
Collaborator

@dgliwka dgliwka commented Nov 10, 2020

No description provided.

Copy link
Owner

@allgreed allgreed left a comment

Choose a reason for hiding this comment

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

Very nice work -> clear and concise! If we can only move the UI from the core I'd say it's a very merge-worthy PR ;)

Also: (might not be applicable directly here, we'd probably lika a separate patch + some discussion before) I'm starting to abhor this root/non-root distinction in the UI. Like if we could have 2 classes - one regular and one for root - so that the main code is not sprinkled with if root checks ;)

// TODO: how about children being Sets?
children: TreeNode<T>[];
data: T;
lastSelectedChild: NodeID | null;
Copy link
Owner

Choose a reason for hiding this comment

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

Aside for my hatred for nulls -> this most definitely does not belong in the core, since it's a UI matter => but the same logic injected into UI Node would de most definitely sensible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants