Skip to content

Feat: add support for circular text background shape #3386

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

Merged

Conversation

jquery404
Copy link
Contributor

@jquery404 jquery404 commented Jun 11, 2025

Cross-references to related issues.
⚠️ Currently, label backgrounds in Cytoscape.js are limited to rectangle and round-rectangle.

Associated issues:

Notes re. the content of the pull request. Give context to reviewers or serve as a general record of the changes made. Add a screenshot or video to demonstrate your new feature, if possible.

  • This PR adding support for text-background-shape: circle
  • Also fixes some redundant drawing issues with rectangle and round-rectangle.

📸 Demo:
Screenshot 2025-06-11 at 6 24 31 PM
Screenshot 2025-06-11 at 6 24 35 PM

Checklist

Author:

  • The proper base branch has been selected. New features go on unstable. Bug-fix patches can go on either unstable or master.
  • Automated tests have been included in this pull request, if possible, for the new feature(s) or bug fix. Check this box if tests are not pragmatically possible (e.g. rendering features could include screenshots or videos instead of automated tests).
  • The associated GitHub issues are included (above).
  • Notes have been included (above).
  • For new or updated API, the index.d.ts Typescript definition file has been appropriately updated.

Reviewers:

  • All automated checks are passing (green check next to latest commit).
  • At least one reviewer has signed off on the pull request.
  • For bug fixes: Just after this pull request is merged, it should be applied to both the master branch and the unstable branch. Normally, this just requires cherry-picking the corresponding merge commit from master to unstable -- or vice versa.

cc: @maxkfranz @chrtannus

@maxkfranz
Copy link
Member

How does this work if you have a long, single-line label? All the existing bounds logic for labels assumes that the bounds are a rectangle.

@maxkfranz maxkfranz requested review from Copilot and maxkfranz June 12, 2025 14:17
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for a new circular text background shape and fixes redundant drawing issues for rectangle and round-rectangle backgrounds.

  • Adds "circle" as an allowed value in the text background shape property.
  • Introduces a new canvas drawing function for circular backgrounds and refines the round rectangle drawing logic.
  • Updates documentation and tests to reflect the new background shape.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

File Description
src/style/properties.mjs Added the "circle" option to the textBackgroundShape enums.
src/extensions/renderer/canvas/drawing-label-text.mjs Introduced the circle drawing function and adjusted drawing logic for background shapes.
documentation/md/style.md Updated the documentation to include "circle" for text backgrounds.
debug/tests.js Added a test case for the new circular text background shape.
Comments suppressed due to low confidence (1)

src/extensions/renderer/canvas/drawing-label-text.mjs:132

  • Consider adding a documentation comment for the 'circle' function to briefly explain its purpose and parameters.
function circle(ctx, x, y, width, height) {

@jquery404
Copy link
Contributor Author

jquery404 commented Jun 12, 2025

How does this work if you have a long, single-line label? All the existing bounds logic for labels assumes that the bounds are a rectangle.

text-background-shape: circle is more of a visual-only style for short or square-shaped labels. The bounding box remains rectangular even for a long, single-line label
image

This’ll be useful for use cases like
image

@maxkfranz
Copy link
Member

maxkfranz commented Jun 16, 2025

That's fine. Those sorts of details would need to be noted in the documentation, so that the user's expectation is aligned with how it works.

Would these circles be compatible with text-background-padding?

@jquery404
Copy link
Contributor Author

That's fine. Those sorts of details would need to be noted in the documentation, so that the user's expectation is aligned with how it works.

Happy to add that

Would these circles be compatible with text-background-padding?

Yes, it does. In fact, padding allows the circle to expand.

@maxkfranz
Copy link
Member

Great, this should be good to merge in with the documentation tweak

@jquery404
Copy link
Contributor Author

@maxkfranz, documentation has been updated - would be great to get it merged. Cheers!

@maxkfranz maxkfranz linked an issue Jun 17, 2025 that may be closed by this pull request
4 tasks
@maxkfranz maxkfranz merged commit d343275 into cytoscape:unstable Jun 17, 2025
1 check passed
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.

Add support for circular text background shape
2 participants