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

`eles.boundingBox()` options : `includeMainLabels`, `includeSourceLabels`, `includeTargetLabels` #2502

Closed
maxkfranz opened this issue Aug 16, 2019 · 1 comment

Comments

@maxkfranz
Copy link
Member

commented Aug 16, 2019

Issue type

Feature request

Description of new feature

Add the following options to eles.boundingBox() (and eles.renderedBoundingBox()):

  • includeMainLabels : A boolean, whether to include the main label in the returned bounds.
  • includeSourceLabels : A boolean, whether to include the source label of an edge in the returned bounds.
  • includeTargetLabels : A boolean, whether to include the target label of an edge in the returned bounds.

Behaviour to maintain compatibility with existing apps:

  • The existing includeLabels option acts as an override when false, excluding all the labels in the bounding box result.
  • Each flag is true by default.

Motivation for new feature

See #1679. If you can query an element for only the bounding box of a particular label, then it's easy to determine whether an event.position is within the bounds of the label.

By using this feature, a programmer can add features like direct manipulation of labels (e.g. drag on a label to move it). It also allows for features like click-to-type. Those sorts of features would make great extensions.

This proposed feature enables these use-cases with minimal added complexity of the Cytoscape.js API.

@maxkfranz maxkfranz added this to the 3.10.0 milestone Aug 16, 2019
@maxkfranz maxkfranz changed the title `eles.boundingBox()` options : `includeLabel`, `includeSourceLabel`, `includeTargetLabel` `eles.boundingBox()` options : `includeMainLabels`, `includeSourceLabels`, `includeTargetLabels` Aug 30, 2019
@maxkfranz

This comment has been minimized.

Copy link
Member Author

commented Aug 30, 2019

The original spec has been updated to be more consistent. Each option is plural.

maxkfranz added a commit that referenced this issue Aug 30, 2019
…els`, `includeTargetLabels` #2502

- Add support for the new options.
- Add documentation.
- Add visual testing support in the debug page.  This feature isn't testable headlessly.
@maxkfranz maxkfranz closed this Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.