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

Remove activeGroup functionalities from Group, create ActiveSelection class #4076

Merged
merged 21 commits into from
Jul 22, 2017

Conversation

asturur
Copy link
Member

@asturur asturur commented Jul 5, 2017

Pro:
Simplifications:
getActiveObject, setActiveObject, getActiveObjects return an array of selected objects.
getActiveGroup, setActiveGroup are removed.
deactivateAllWithDispatch is removed, use discardActiveGroup.
deactivateAll is removed, use _discardActiveGroup.
Added activeSelection => group, single high level method.
removed all the dancing of originalHasControls, hasControls relative to group.
removed the weird logic of rendering controls in groups.
built in possibility to cancel a deselect, making onDeselect return true.
possibility to do group => activeSelection

missing:
restoring the lock system, an object locked in the active selection makes the activeselection locked

Cons:
Api changes
everyone confused on what is going on
possible regressions
riot
i eventually get killed

@asturur asturur mentioned this pull request Jul 17, 2017
@asturur asturur merged commit c9b562d into master Jul 22, 2017
@asturur asturur deleted the test-new-group branch July 22, 2017 21:05
@ozooner
Copy link
Contributor

ozooner commented Sep 28, 2017

I hope you won't get killed any time soon 😃

In case someone happens to read this, few corrections:

deactivateAllWithDispatch is removed, use discardActiveGroup.

use discardActiveObject

deactivateAll is removed, use _discardActiveGroup.

use _discardActiveObject

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.

2 participants