-
-
Notifications
You must be signed in to change notification settings - Fork 879
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
[Feature] configure custom line-drawing method #603
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
感谢支持,很好的功能。我提了点儿想法,麻烦你看一下。谢谢!
src/jsmind.graph.js
Outdated
@@ -20,7 +20,11 @@ class SvgGraph { | |||
straight: this._line_to, | |||
curved: this._bezier_to, | |||
}; | |||
this.drawing = this.line_drawing[this.opts.line_style] || this.line_drawing.curved; | |||
if (typeof this.opts.line_style === 'function') { | |||
this.drawing = this.opts.line_style; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我们需要考虑如何兼容两种不同的绘图模式,SVG 模式下这个参数如果是适用于 Canvas 的方法怎么办?
从 API 的角度,我觉得再加一个参数比较好,比如叫 custom_node_render. 用一个 failSafe 的方法包装它,有异常的话可以回落到默认的逻辑里。
default_drawing = this.line_drawing[this.opts.line_style] || this.line_drawing.curved;
if (typeof this.opts.line_style === 'function') {
this.drawing = function(params...){
try {
this.opts.line_style(params...);
} catch {
log...
default_drawing(params...);
}
}
} else {
this.drawing = default_drawing
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{
engine: 'canvas', // 思维导图各节点之间线条的绘制引擎
line_style:'curved',// 思维导图线条的样式,直线(straight)或者曲线(curved)
custom_canvas_line_style:()=>{}, // 自定义 canvas 绘制线条, 此参数会覆盖 line_style, 且当 engine: 'canvas' 时生效
custom_svg_line_style:()=>{}, // 自定义 svg 绘制线条, 此参数会覆盖 line_style, 且当 engine: 'svg' 时生效
}
这样如何
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
个人认为此功能的代码兼容性不应该在框架解决, 这是自定义功能, 应让用户自行检查编码问题, 如果绘制出错导致无线条显示也可以更好的判断是代码问题, 而不是回落到曲线误认为自定义未生效.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我其实能理解你的想法,我在这里也纠结了很久,一方面不想设计得过于复杂,另一方面又不想过于死板。不过后来我想通了:
- 对于使用
jsmind
的 user 来说,他应该不需要经常切换这两种模式,所以选择一种模式之后,基于这种模式做配置是没有问题的 - 对于
jsmind
的 contributer 来说,让 user 能使用 jsmind library 实现其业务功能就OK了,无论是提供 custom 方法也好,还是提供更多的 line_style 都是 OK 的
engine 目前是个参数,如果我们定义两个方法,那相当于在枚举所有的选项,相当于把 user 看作了 contributor ,而给他增加难度了。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
个人认为此功能的代码兼容性不应该在框架解决, 这是自定义功能, 应让用户自行检查编码问题, 如果绘制出错导致无线条显示也可以更好的判断是代码问题, 而不是回落到曲线误认为自定义未生效.
同意这个原则,上面这个思路主要是为了那个 log,回落到默认曲线只是随手的事,另外使用 try...catch... 来确保其它代码能顺利执行,从而让 error log 里不会出现过多的干扰。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那我加上try catch 和 log, 就不回落了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
加了点儿意见,请再看一下。谢谢。
docs/en/2.options.md
Outdated
@@ -38,7 +38,7 @@ options = { | |||
vmargin:50, // Minimum vertical distance of the mindmap from the outer frame of the container | |||
line_width:2, // thickness of the mindmap line | |||
line_color:'#555', // Thought mindmap line color | |||
line_style:'curved', // line style, straight or curved | |||
line_style:'curved', // line style, straight or curved, or a callback function presents controlled by user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
用一个新的参数吧,因为在function里其实不仅能修改 style,还能修改 width 和 color
src/jsmind.graph.js
Outdated
pout.x + offset.x, | ||
pout.y + offset.y | ||
); | ||
} catch (e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可以在声明 drawing 方法的时候处理 try...catch, 因为我们并不需要处理默认的 drawing 方法的异常,而只应该把 custom 的部分放到 try...catch...里。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这样的话,log 里可以明确说明是 custom 方法产生的问题。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class CanvasGraph {
constructor(view) {
this.opts = view.opts;
this.e_canvas = $.c('canvas');
this.e_canvas.className = 'jsmind';
this.canvas_ctx = this.e_canvas.getContext('2d');
this.size = { w: 0, h: 0 };
this.line_drawing = {
straight: this._line_to,
curved: this._bezier_to,
};
this.init_line_style();
}
init_line_style(){
if (typeof this.opts.custom_line_render === 'function') {
this.drawing = (ctx,x1,y1,x2,y2)=> { //svg: path,x1,y1,x2,y2
try {
this.opts.custom_line_render.call(this, {ctx,start_point:{x:x1,y:y1},end_point:{x:x2,y:y2}});
//svg: this.opts.custom_line_render.call(this, {ctx:path,start_point:{x:x1,y:y1},end_point:{x:x2,y:y2}});
} catch (e) {
logger.error('custom line render error: ',e);
}
}
} else {
this.drawing = this.line_drawing[this.opts.line_style] || this.line_drawing.curved;
}
}
}
我将其这样设计, 怎么样,
参数的话有两种选择:
- svg的custom_line_render的参数和canvas形式一样, path也叫ctx,
- svg叫path, canvas叫ctx
配置:
{
line_width:2, // 思维导图线条的粗细
line_color:'#555', // 思维导图线条的颜色
line_style:'curved',// 思维导图线条的样式,直线(straight)或者曲线(curved)
custom_line_render: null, // 自定义的线条样式渲染方法, 在此方法中可覆盖粗细和颜色
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这样设计很好。
参数名选哪个方案我觉得问题不大,改不改都行。
另外你提PR前在本地跑一下 npm run format
刷一下格式,这样 CI 就能过了。
没有要更改的内容的话我就修改文档了 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
感谢!
docs/en/2.options.md
Outdated
@@ -130,10 +131,16 @@ These options are described in more detail below. | |||
**view.line_color** : (string) color of the mindmap line (color representation in HTML). If `data.leading-line-color` is set on `node`, this option will be overridden. | |||
> By default, the lines are 2px in thickness and dark gray in color (#555). | |||
|
|||
**view.line_style** : (string) style of the mindmap line. The available options are: | |||
**view.line_style** : (string|function) style of the mindmap line. The available options are: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
删除 function
src/jsmind.graph.js
Outdated
} | ||
static c(tag) { | ||
return $.d.createElementNS('http://www.w3.org/2000/svg', tag); | ||
} | ||
init_line_style() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init_line_render
Line style controlled by user.
Replace
line_style
option with a function to draw a custom line.docs in this PR.