Skip to content

Associate Action with right Partition #3224

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Apr 6, 2024
Merged

Associate Action with right Partition #3224

merged 16 commits into from
Apr 6, 2024

Conversation

amolenaar
Copy link
Member

PR Type

What kind of change does this PR introduce?

  • Bug fix
  • Feature
  • Chore (refactoring, formatting, local variables, other cleanup)
  • Documentation content changes

What is the current behavior?

Actions are always assigned to the first partition when dragging.

Issue Number: fixes #3050

What is the new behavior?

The partition below the mouse cursor is now found.

Other information

  • Add a bit of extra typing to the core collection class.
  • Added tests for State machine dropzone behavior.

@amolenaar
Move drop functionality to separate method
53293b1 
So we only have to override one.
@amolenaar
Add dropzone support for partitions
a8a588a
@amolenaar
Also add test for State regions
f518deb 
I think this is a nice example of what happens if you
do not build your software with TDD. It actually needs
a style sheet to work!
@amolenaar
Add view fixture to toplevel conftest
b77a3cd
@amolenaar
Add an extra bit of typing to core collection
54d17e6
@amolenaar amolenaar requested a review from danyeaw March 30, 2024 20:14
@github-actions github-actions bot added the python Pull requests that update Python code label Mar 30, 2024
@amolenaar amolenaar changed the title Action on right partition Associate Action with right Partition Mar 30, 2024
@danyeaw danyeaw added fix A fix for a bug and removed python Pull requests that update Python code labels Mar 31, 2024
@danyeaw
Merge branch 'main' into action-in-partition
33b7a59 
# Conflicts:
#	gaphor/diagram/tests/test_instanteditors.py
#	gaphor/diagram/tools/tests/conftest.py
@github-actions github-actions bot added the python Pull requests that update Python code label Mar 31, 2024
@danyeaw danyeaw removed the python Pull requests that update Python code label Mar 31, 2024
danyeaw
danyeaw reviewed Mar 31, 2024
View reviewed changes
Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

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

Hi @amolenaar, these changes look good. I gave it a spin and for me the first drop is still being associated with the first swimlane even if I drop it on the 2nd. When I select the action and release it, then it is associated with the 2nd one.

@amolenaar
Copy link
Member Author

amolenaar commented Apr 1, 2024 •
edited
Loading

Ah, I only fixed it for moving actions from one partition to the other.

Did you drop from the model browser? There no grouping is implemented.

Did you drag from the toolbox?

I added a TODO for placement tool. Not sure yet how to do the grouping without duplicating the logic.

@github-actions github-actions bot added the python Pull requests that update Python code label Apr 1, 2024
@danyeaw
Copy link
Member

danyeaw commented Apr 1, 2024

Hi @amolenaar, I was dropping from the toolbox since I thought that would be the primary use case.

amolenaar added 6 commits April 1, 2024 22:30
@amolenaar
Make drop() a multidispatch function
23a0e4c 
So we can use it to drop any element onto any other element.
@amolenaar
Move drop to presentation to drop module
73ab6eb
@amolenaar
Use drop() to move states on diagram
7a4aead
@amolenaar
Implement drop-grouping for actions
7178aa9
@amolenaar
Use drop for new items
757ac3a 
Also add some pre-conditions and deal with placing an item outside of a
container.
@amolenaar
Update docs
056b723
@amolenaar
Copy link
Member Author

amolenaar commented Apr 1, 2024 •
edited
Loading

I extended the use of drop() to also support dropping items on other items in a diagram. This simplifies the code, I think, since we only need one concept (drop) to handle items being placed on top of one another.

You can now create actions directly on a swim lane, and it's placed in the right swimlane.

Somehow this doesn't work that well for state machines. I have not been able to figure out why, yet.

@amolenaar amolenaar requested a review from danyeaw April 1, 2024 22:09
amolenaar added 2 commits April 2, 2024 00:14
@amolenaar
Import drop modules directly
3e38966 
not via gaphor.modules imports.
@amolenaar
Record region heigths separately
578d834 
So that nested states can still be properly placed afte an
`update_shape()` call.
@amolenaar
Copy link
Member Author

I updated the StateItem and StateMachineItem code, so that nested states can be properly placed in the right region.

The problem was that updates caused the shape to be recreated. As a result regions could not be determined.

@danyeaw
Copy link
Member

danyeaw commented Apr 3, 2024

@amolenaar Oh nice one! I haven't had a chance to look at the updates yet, but I should be able to make some time tomorrow to do one last review.

danyeaw
danyeaw approved these changes Apr 6, 2024
View reviewed changes
Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

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

Really nice update to the drop zones @amolenaar!

@danyeaw danyeaw removed python Pull requests that update Python code documentation labels Apr 6, 2024
@danyeaw danyeaw merged commit ebee598 into main Apr 6, 2024
@danyeaw danyeaw deleted the action-in-partition branch April 6, 2024 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danyeaw danyeaw danyeaw approved these changes

Assignees
No one assigned
Labels
fix A fix for a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

First activity partition gets all the nodes
2 participants
@amolenaar @danyeaw