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(graph): Add selection based zoom #248

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

jeff-phillips-18
Copy link
Member

@jeff-phillips-18 jeff-phillips-18 commented Oct 31, 2024

What

Closes #222

Description

Adds the ability to drag an area on the graph.
Added events for:

  • GRAPH_AREA_DRAGGING_EVENT - When user is dragging an area on the graph
  • GRAPH_AREA_SELECTED_EVENT - When the user completed the selection of an area

Adds a hook to allow area selection via withAreaSelection. This takes key modifiers that can be handled separately so applications can do different things based on the shift key, cntrl key, etc.

Adds two methods to the Graph element:

  • zoomToSelection : pass it the values from the GRAPH_AREA_SELECTED_EVENT to zoom to that area
  • nodesInSelection : pass it the values from the GRAPH_AREA_SELECTED_EVENT to get all nodes in that area

The Topology Package demo is updated to add both shift and cntrl drag and to zoom for cntrl and select nodes for shift. Also handles the GRAPH_AREA_DRAGGING_EVENT to show a hint to users on how to perform these actions.

Type of change

  • Feature

Screen shots / Gifs for design review

TopologyAreaSelect

@patternfly-build
Copy link

patternfly-build commented Oct 31, 2024

@jeff-phillips-18 jeff-phillips-18 changed the title WIP: Selection-based zoom feat(graph): Add selection based zoom Nov 1, 2024
@jeff-phillips-18
Copy link
Member Author

Demo surge

@jeff-phillips-18
Copy link
Member Author

@andrew-ronaldson Please let me know your thoughts on using the control button for the interaction as well as the color/opacity of the selection rectangle.

@jshaughn
Copy link
Collaborator

jshaughn commented Nov 1, 2024

Looking great @jeff-phillips-18 !

Copy link
Collaborator

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

The selection based zoom looks awesome @jeff-phillips-18 ! Really great work bringing this cool feature to life 🎉

Just a couple of thoughts that could enhance the flow and documentation:

  • Would it be helpful to add a toggle switch / toolbar item to turn the selection-based zoom on/off? Just to provide a visual cue that tells consumers to use ctrl+click+drag to get this behavior.

  • Currently using ctrl+click+drag over areas that don’t include any part of the node/graph results in an "empty" view:

Screenshot 2024-11-01 at 11 37 38 AM

Wondering we can add some logic that prevents pan/zoom if there are no topology components in the zoom area. WDYT @jeff-phillips-18 ?

@jshaughn
Copy link
Collaborator

jshaughn commented Nov 1, 2024

I'm totally happy with the select-zoom but sort of in the direction @jenny-s51 was commenting on, not sure if anyone would have a need for just the "select" part, getting a handle on the selected elements to do something custom.

@jeff-phillips-18
Copy link
Member Author

Wondering we can add some logic that prevents pan/zoom if there are no topology components in the zoom area

I think it would be odd to do nothing if the user selected an empty area.

We could provide an event when the zoom selection is made and allow applications to handle that event as they see fit. The event could include the extents and the app could then zoom (via a provided method) or select all items within the bounds, or ignore if there are no items within the bounds.

Would it be helpful to add a toggle switch / toolbar item to turn the selection-based zoom on/off? Just to provide a visual cue that tells consumers to use ctrl+click+drag to get this behavior

I'd also say we could add an event when the user pans the graph (click+drag), this would allow applications to add an indicator to use the cntrl key to select an area. Similar to what OpenShift Console does for dragging nodes out of groups:

openshift-topology-drag-node

I'd like to hear @andrew-ronaldson 's thoughts on this.

@andrew-ronaldson
Copy link
Collaborator

For visual cues there could be a ? icon in the control bar with shortcuts info in a popover/modal. I remember OCP had the view shortcuts link button in the top bar too. I liked the indicator in your recording @jeff-phillips-18 to communicate what shift + drag was doing. Is it a fixed position for that tooltip?

Love this addition and I think there are some other useful controls we could add later. For example holding down spacebar in Miro and Adobe apps gives you a hand icon to click & drag on the canvas. That is useful when there are lots of groups on the page that you may inadvertently drag something when you just want to move around the board.

@jeff-phillips-18
Copy link
Member Author

@andrew-ronaldson @jshaughn @jenny-s51 I've updated the PR description and the surge. PTAL.

Demo surge

Still trying to finalize the color/opacity for the selection area box.

@jshaughn
Copy link
Collaborator

@jeff-phillips-18 very cool, love the option to zoom OR grab the selected nodes to do whatever one may want to do. Would love to see this in PFT5.

@jeff-phillips-18
Copy link
Member Author

@andrew-ronaldson updated selection area color and opacity as suggested. Please take a look at the Updated Surge

@jeff-phillips-18
Copy link
Member Author

@jenny-s51 @dlabaj This is ready for review.

Copy link
Collaborator

@jenny-s51 jenny-s51 left a comment

Choose a reason for hiding this comment

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

Really great work on this new feature @jeff-phillips-18 ! Hint component provides helpful visual cues and drag area looks good. All drag / selection behaviors work as expected as well. LGTM

@jeff-phillips-18
Copy link
Member Author

AreaSelection

Copy link
Collaborator

@andrew-ronaldson andrew-ronaldson left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! I'll work on the v6 tokens

@jeff-phillips-18 jeff-phillips-18 merged commit ad0be9c into patternfly:v5 Nov 13, 2024
8 checks passed
@jeff-phillips-18 jeff-phillips-18 deleted the selection-zoom branch November 13, 2024 13:35
@jeff-phillips-18
Copy link
Member Author

@jshaughn Sorry for the delay in release. @dlabaj is working on a fix for Semantic release is broken on v5

@jeff-phillips-18
Copy link
Member Author

Released in v5.4.1

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.

5 participants