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

Feat/editable-label #126

Merged
merged 36 commits into from
Apr 29, 2024
Merged

Feat/editable-label #126

merged 36 commits into from
Apr 29, 2024

Conversation

dontry
Copy link
Collaborator

@dontry dontry commented Mar 12, 2024

This MR enables user to edit message labels.

  • Create new component <MessageLabel /> and use it in Message and SelftInvocation component
  • Create a reusable hook useEditLabel containing event handlers for editing labels.
  • Refactor <ConditionLabel /> to use useEditLabel hook
  • Add a new regex removeSpacesBeforeAndInsideBrackets for getFormattedText function
  • Add test cases to cypress e2e test
  • Update github actions e2e.yml to fix Python dependencies missing issue

TODO: fix overflow issue of the first method label upon first time edit

@MrCoder
Copy link
Collaborator

MrCoder commented Mar 17, 2024

Layout is not updated:

image image

Copy link
Collaborator

@MrCoder MrCoder left a comment

Choose a reason for hiding this comment

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

Layout is not updated. See images in comments.

@dontry dontry marked this pull request as draft March 17, 2024 22:22
@dontry dontry marked this pull request as ready for review March 23, 2024 10:51
@dontry dontry self-assigned this Mar 27, 2024
@dontry dontry requested a review from MrCoder March 27, 2024 10:18
@dontry dontry changed the title Feat/ediable-label Feat/editable-label Mar 28, 2024
@MrCoder
Copy link
Collaborator

MrCoder commented Mar 31, 2024

It does not wrap the method with double quotes if there are spaces or special characters.

image

@MrCoder MrCoder mentioned this pull request Apr 2, 2024
3 tasks
@MrCoder
Copy link
Collaborator

MrCoder commented Apr 4, 2024

修改message之后,没有刷新Layout
image

@dontry
Copy link
Collaborator Author

dontry commented Apr 5, 2024

修改message之后,没有刷新Layout image

没有办法复现问题。稍微做了一些优化改动。

@MrCoder
Copy link
Collaborator

MrCoder commented Apr 17, 2024

只有在特定的DSL下才可以重现。下面的示例,修改methodLong2

A.method1 {
  B.methodLong2
}
Screen.Recording.2024-04-17.at.10.19.16.PM.mov

@dontry
Copy link
Collaborator Author

dontry commented Apr 18, 2024

修改message之后,没有刷新Layout image

这个问题是因为我掉到一个坑。之前定义的regex有一个g代表全局搜索,是有状态的。造成第一次碰到特殊字符能识别,之后搜索是从上一次得到的index算起,导致后面的检测不成功。现在这个bug已经修复了。其他你碰到的问题,我没发复现。你试试再拉一下最新的commit测试一下?有问题的情况,可以附上DSL, 我可以做regression test

@dontry
Copy link
Collaborator Author

dontry commented Apr 18, 2024

我决定添加一些e2e测试到smoke test

@MrCoder
Copy link
Collaborator

MrCoder commented Apr 20, 2024

Spaces in selfMessage is not handled properly.

Screen.Recording.2024-04-20.at.5.52.27.PM.mov

@dontry
Copy link
Collaborator Author

dontry commented Apr 28, 2024

Similar issues on Self message. See the last comment.

Fixed

src/core.ts Outdated
@@ -104,7 +104,6 @@ export default class ZenUml implements IZenUml {
async doRender(config: Config | undefined) {
console.debug("rendering start");
const start = getStartTime();
clearCache();
Copy link
Collaborator

Choose a reason for hiding this comment

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

@danshuitaihejie 这个地方删掉有没有别的影响?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@danshuitaihejie 这个地方删掉有没有别的影响?

这个地方不能的clearCache不能删除

@MrCoder MrCoder merged commit 445f884 into main Apr 29, 2024
12 checks passed
@MrCoder MrCoder deleted the feat/ediable-label branch April 29, 2024 04:21
@@ -7,7 +7,7 @@
ref="messageRef"
>
<label
class="name group px-px hover:text-skin-message-hover hover:bg-skin-message-hover relative min-h-[1em]"
class="name group px-px hover:text-skin-message-hover hover:bg-skin-message-hover relative min-h-[1em] w-full"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is causing an undesired effect see below video. This does not seem to be the cause if the issue. There is real issue that needs to be fixed. The corresponding interaction should not have width at the first place.

self-message-centred.mp4

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.

None yet

3 participants