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.

Sign up for GitHub

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

Jump to bottom

refactor: use react hooks and typescript refactor #19

Merged
merged 8 commits into from
Jul 19, 2020
Merged

refactor: use react hooks and typescript refactor #19

merged 8 commits into from
Jul 19, 2020

Conversation

akitaSummer
Copy link
Collaborator

我帮您优化了一点样式
https://ibb.co/LP0DrfB
改后
https://ibb.co/zfjj2Jw

@AGDholo
Copy link
Member

AGDholo commented Jul 17, 2020

Result 这个单词没有和下面的输出框对齐

@lidangzzz
Copy link
Member

@AGDholo 惊了，这个我不知道该不该approve，你觉得呢？

cc： @akitaSummer

@AGDholo
Copy link
Member

AGDholo commented Jul 17, 2020

@lidangzzz 他后面 force-pushed 的代码移除了左边距（看代码应该是已经对齐了），然后整体代码的效果应该是将 result 和左边卡片的标题进行了垂直的居中对齐，要不要 approve 就得看你的喜好了。

另外这个 commit 用了 yarn，我刚刚才把 yarn 文件移除（因为我看之前源码的提交的包管理都是 npm 来操作的，yarn 没怎么用，所以我上个 pr 把 yarn 文件删了），最好能统一成 npm 吧

@akitaSummer
Copy link
Collaborator Author

已经修改好了
https://ibb.co/0XGw9J6
当没有数据的时候，是不是应该取消掉Textarea，改用
https://ibb.co/7j4GyjR
这样的提示较好呢

@akitaSummer
Copy link
Collaborator Author

以及第三个样例在点击运行时有明显卡顿，我推测您应该没有使用Web Workers导致的，如果能将计算用Web Workers进行，并将Compile and run按钮变成loading状态，应该会有更好的用户体验

@lidangzzz
Copy link
Member

@akitaSummer 之前想用web worker，然而没用的原因是只能传入script file，我还要看看怎么用blob传入一段script。

这个pull request我再看看，我还不太会用git（另外你们知道如何用tortoiseHg管github repo吗）

@akitaSummer
Copy link
Collaborator Author

我这里帮您把web worker已经配置进去了，不过会有两个问题，我在代码的注释里写到了，可以看一下
button的loading我已经添加上了
https://ibb.co/cFQJRmc

@AGDholo
Copy link
Member

AGDholo commented Jul 18, 2020

刚刚合并了 #22 ，现在目录和文件内容都改变了，代码冲突估计很严重，可能需要重新拉取然后 commit

@lidangzzz
Copy link
Member

@AGDholo @akitaSummer 真的很抱歉，斯米马赛，我刚才尝试手动解决可惜我对git并不熟练。另外我自己原本也希望1.0版本之后再让大家帮忙贡献，没想到大家这么积极，我没有管理好各个branch和versioning。我先晚上试着解决一下conflicts，解决不了咱们再想办法

@akitaSummer
Copy link
Collaborator Author

我这里已经修复了冲突，您可以branch一个新的分支，我合并到那个分支上，您checkout看下效果

imtsuki
imtsuki reviewed Jul 18, 2020
View reviewed changes
hedgehog-lab/src/core/output/output-item.js
Comment on lines +12 to +17
// use arrow function will make web worker error
// isPrint = () => this.outputType === 'print';
// isDraw = () => this.outputType === 'draw';
// isTex = () => this.outputType === 'tex';
// isFormulaTex = () => this.outputType === 'formulaTex';
// isMarkdown = () => this.outputType === 'markdown';
Copy link
Member

Choose a reason for hiding this comment

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

这里估计是 webpack 的问题

imtsuki
imtsuki reviewed Jul 18, 2020
View reviewed changes
hedgehog-lab/package.json Outdated
@@ -60,7 +60,10 @@
]
},
"devDependencies": {
"@babel/standalone": "^7.10.2",
Copy link
Member

Choose a reason for hiding this comment

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

这个依赖重复了

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

抱歉，没注意到，已修复

@akitaSummer
Copy link
Collaborator Author

hi，我用react hooks和ts重构了你们的react部分，我还希望能够修改一些你们UI相关的功能，但是我的代码你们还没有merge，可能再更改下去会越来越难合并，你们要不要先开一个分支合一下我的看看是否符合你们的预期。 @lidangzzz @AGDholo @imtsuki

@AGDholo AGDholo mentioned this pull request Jul 18, 2020
Closed
@AGDholo AGDholo mentioned this pull request Jul 19, 2020
Closed
@AGDholo
Copy link
Member

AGDholo commented Jul 19, 2020 •
edited

@akitaSummer 我刚刚解决了冲突。
发现个 bug，Tutorial 4: Built-in functions 这个 example 编译出来的是一堆 object

[object Object]
[object Object]
[object Object]

This was referenced Jul 19, 2020
Closed
Closed
@lidangzzz
Copy link
Member

@AGDholo 好的，我先看一下

@AGDholo AGDholo added the enhancement New feature or request label Jul 19, 2020
This was linked to issues Jul 19, 2020
把计算扔WebWorker里做 #7
Closed
Use ts (or tsx) extension for .js files with type annotations (or tsx syntax) #30
Closed
@AGDholo AGDholo changed the title fix: fix title style refactor: use react hooks and typescript refactor Jul 19, 2020
@lidangzzz
Copy link
Member

@AGDholo @akitaSummer 我觉得这个很好了，请问我现在可以merge了不- -

@lidangzzz lidangzzz merged commit a1854ce into Hedgehog-Computing:master Jul 19, 2020
@lidangzzz
Copy link
Member

好了我merge完了

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

@imtsuki imtsuki imtsuki left review comments

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use ts (or tsx) extension for .js files with type annotations (or tsx syntax) 把计算扔WebWorker里做
4 participants
@akitaSummer @AGDholo @lidangzzz @imtsuki