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: connection highlight zelos #7781

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented Jan 11, 2024

The basics

The details

Resolves

Work on #7204

Proposed Changes + reasons

Makes it so that zelos has the ability to highlight connections, so the eventual connection previewer system can call the renderer to highlight connections consistently (instead of sometimes calling the renderer, and sometimes doing other things).

Test Coverage

Manually tested highlighting all three shapes of dynamic connections in zelos (hexagon, rounded, and square). Manually tested highlighting non-dynamic connections (puzzle tabs).

Documentation

N/A

Additional Information

N/A

To Review

The drawer architecture is... a bit frustrating. All of the methods are stateful, so I had to pull out some logic for creating internal input paths into pure methods. This way the code could be reused.

Some of the "constants" for path shapes were also broken. Previously pathDown() was never being called, so no one noticed that it was busted. I fixed up the logic to actually work for all combinations of right or left side + drawing up or down.

[Edit: docs for context https://developers.google.com/blockly/guides/create-custom-blocks/renderers/create-custom-renderers/connection-shapes#basic_shapes]

@BeksOmega BeksOmega requested a review from a team as a code owner January 11, 2024 18:45
@BeksOmega BeksOmega requested review from cpcallen and removed request for a team January 11, 2024 18:45
@github-actions github-actions bot added PR: feature Adds a feature and removed PR: feature Adds a feature labels Jan 11, 2024
@BeksOmega BeksOmega force-pushed the feat/connection-highlight-zelos branch from 6a1e65a to 3d32f1e Compare January 11, 2024 18:50
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

I wish this was easier to understand (without extensive familiarity with the renderers) but alas I don't see any obvious improvements to suggest.

@github-actions github-actions bot added PR: feature Adds a feature and removed PR: feature Adds a feature labels Jan 11, 2024
@BeksOmega
Copy link
Collaborator Author

Thank you for the review Christopher! :D

@BeksOmega BeksOmega merged commit 6e56aaa into google:connection-previewers Jan 12, 2024
8 checks passed
BeksOmega added a commit to BeksOmega/blockly that referenced this pull request Feb 2, 2024
* feat: add connection highlighting to zelos

* fix: drawing outputs

* chore: cleanup
BeksOmega added a commit that referenced this pull request Feb 2, 2024
* feat: add connection highlighting to zelos

* fix: drawing outputs

* chore: cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: feature Adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants