Skip to content

fix: edge handlers use style.edgeStyle #78

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
Merged
tbouffard merged 3 commits into from
Feb 20, 2022
Merged

fix: edge handlers use style.edgeStyle #78

tbouffard merged 3 commits into from
Feb 20, 2022

Conversation

@cd-yang
Copy link

@cd-yang cd-yang commented Feb 13, 2022 •
edited by tbouffard
Loading

Summary
Property 'edge' does not exist on type 'CellStateStyles', which is used in file 'ElbowEdgeHandler' and 'EdgeHandler'

Description for the changelog
Added 'edge' property to 'CellStateStyles'.
Use the edgeStyle property of CellStateStyles in ElbowEdgeHandler and EdgeHandler.

tbouffard
tbouffard requested changes Feb 18, 2022
View reviewed changes
Copy link
Member

@tbouffard tbouffard left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The GitHub action didn't detect the issue earlier but I can see the tsc error in my IDE 😢
So thanks @cd-yang for the detection of the problem and your contribution!

The CellStateStyles edge property has been renamed into edgeStyle in https://github.com/maxGraph/maxGraph/pull/77/files#diff-4c2458380e50c967c70d7afe819dc7412180f9f817043f27441de3498e4d0479
So EdgeHandler and ElbowEdgeHandler should be updated instead of using a new property that is never set elsewhere.

@tbouffard
Copy link
Member

tbouffard commented Feb 18, 2022

I have been able to detect the issue by also running tsc -p packages/core/tsconfig.json from the project root, so I suggest we run this command as part of the GH Actions build.

@cd-yang
Copy link
Author

cd-yang commented Feb 18, 2022 •
edited
Loading

I removed edge from CellStateStyles

@tbouffard tbouffard changed the title fix CellStateStyles fix: edge handlers use style.edgeStyle Feb 18, 2022
@cd-yang cd-yang requested a review from tbouffard February 19, 2022 03:29
@tbouffard
Copy link
Member

tbouffard commented Feb 20, 2022

Thanks @cd-yang for the changes, this looks fine now.
I am first going to create a Pull Request to reproduce the TSC errors with the GitHub Actions. Then I will merge your Pull Request which should fix the issue.
That way, we will ensure that the TSC errors won't appear without prior notice.

@tbouffard tbouffard self-assigned this Feb 20, 2022
@tbouffard tbouffard mentioned this pull request Feb 20, 2022
Merged
@tbouffard tbouffard merged commit 7fa2b43 into maxGraph:development Feb 20, 2022
@aminosbh aminosbh mentioned this pull request Jun 4, 2022
Closed
14 tasks
@tbouffard tbouffard added the bug Something isn't working label Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@junsikshim junsikshim junsikshim approved these changes

@tbouffard tbouffard tbouffard approved these changes

Assignees

@tbouffard tbouffard

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cd-yang @tbouffard @junsikshim