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

Исправлен перенос на ts. Логика дерева отделена от изображения #4

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

Karkh-Elizaveta
Copy link

Указаны типы, использованы стрелочные функции вместо анонимных, let исправлен на cost, где необходимо.
Логика дерева отделена от изобрадения дерева.

src/TreeNode.ts Outdated Show resolved Hide resolved
src/TreeNode.ts Outdated Show resolved Hide resolved
src/App.tsx Outdated
return (
() => <div id={'my-canvas'}></div>
() => <div id={'my-canvas'}/>
Copy link
Member

Choose a reason for hiding this comment

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

Внести идентификатор my-canvas внутрь компонента, отрисовывающего дерево.
Предлагаю сделать ещё один класс-компонент, в котором в render будет возвращаться этот контейнер с внутренним идентификатором, плюс там же будет код который с помощью d3 отвечает за отрисовку.
И уже он будет использовать твой класс Tree.ts для работы с логикой, с моделью графа, который рисуется.

src/TreeNode.ts Outdated
const isError: boolean = this.checkVertex(node, label);
node.children.push(TreeNode.getInstance({
label,
graph: getSubgraph(vertexArray, node.graph),
Copy link
Member

Choose a reason for hiding this comment

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

Чем отличается семантика методов данного класса и функций из модуля 'graph'? Может быть можно избавиться от модуля и сделать функции методами данного класса?

src/TreeNode.ts Outdated Show resolved Hide resolved
src/TreeNode.ts Outdated
return false;
}
else {
Notifier.send({
Copy link
Member

Choose a reason for hiding this comment

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

Предлагаю выбрасывать наверх событие, что произошло в дереве, а сообщение нотификатором отправлять уже из приложения. Если мы будем переиспользовать этот компонент в других местах, чтобы не было завязано на конкретных сообщениях и штрафах.

src/tree.ts Outdated Show resolved Hide resolved
this.redraw();
tree.currLbl = i;
d3.select("#labelpos").text(this.currLbl + '/' + this.glabels.length);
}

private redraw() {
Copy link
Member

Choose a reason for hiding this comment

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

Вынести тоже в компонент визуализации предлагаю, чтобы разделить логику и отрисовку.

@eakarpov eakarpov added the enhancement New feature or request label Mar 25, 2019
@eakarpov eakarpov added this to the Версия 1.0.0 milestone Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants