search

patternfly / react-topology

MIT License
10 stars 19 forks source link

added property to set whether the outline is hull or rect #114

Closed leandroberetta closed 9 months ago

leandroberetta commented 10 months ago

Closes https://github.com/patternfly/react-topology/issues/113

Description

A new property in the DefaultGroup was added to set whether the outline of the group is hulled or rect.

Type of change

Screen shots / Gifs for design review

jeff-phillips-18 commented 10 months ago

@jenny-s51 @nicolethoen Could you PTAL?

jeff-phillips-18 commented 10 months ago

Nit: I'm also wondering since the hulled outline was already the default and these changes introduce the rectangular outline, should we rename it to something like rectOutline instead? I think semantically this makes more sense and we could just set rectOutline instead of setting manually the prop value to false, however this isn't a blocker.

I'm in favor of the hulledOutline just sounds better to me even if it is the default but I could go either way.

leandroberetta commented 10 months ago

Thank you for your contribution @leandroberetta ! Visually this is looking good - just a couple of comments:

It looks like when shift+clicking+dragging a node, the default behavior should move it out of a group. When this new prop is added, I can no longer seem to move the node out of the group. Is this to be expected?

Nit: I'm also wondering since the hulled outline was already the default and these changes introduce the rectangular outline, should we rename it to something like rectOutline instead? I think semantically this makes more sense and we could just set rectOutline instead of setting manually the prop value to false, however this isn't a blocker.

cc @jeff-phillips-18 what do you think?

@jenny-s51, thanks for the review, @jeff-phillips-18 helped me with the issue you reported.

github-actions[bot] commented 9 months ago

:tada: This PR is included in version 5.2.0-prerelease.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket: