Skip to content

DragControls: Refactor API. #29079

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

Merged
merged 6 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 17 additions & 30 deletions docs/examples/en/controls/DragControls.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<link type="text/css" rel="stylesheet" href="page.css" />
</head>
<body>
[page:EventDispatcher] &rarr;
[page:Controls] &rarr;

<h1>[name]</h1>

Expand Down Expand Up @@ -95,42 +95,44 @@ <h3>hoveroff</h3>

<h2>Properties</h2>

<h3>[property:Boolean enabled]</h3>
<p>
Whether or not the controls are enabled.
</p>
<p>See the base [page:Controls] class for common properties.</p>

<h3>[property:Boolean recursive]</h3>
<h3>[property:Array objects]</h3>
<p>
Whether children of draggable objects can be dragged independently from their parent. Default is `true`.
An array of draggable 3D objects.
</p>

<h3>[property:Boolean transformGroup]</h3>
<h3>[property:Raycaster raycaster]</h3>
<p>
This option only works if the [page:DragControls.objects] array contains a single draggable group object.
If set to `true`, [name] does not transform individual objects but the entire group. Default is `false`.
The internal raycaster used for detecting 3D objects.
</p>

<h3>[property:String mode]</h3>
<h3>[property:Boolean recursive]</h3>
<p>
The current transformation mode. Possible values are `translate`, and `rotate`. Default is `translate`.
Whether children of draggable objects can be dragged independently from their parent. Default is `true`.
</p>

<h3>[property:Float rotateSpeed]</h3>
<p>
The speed at which the object will rotate when dragged in `rotate` mode. The higher the number the faster the rotation. Default is `1`.
</p>

<h3>[property:Boolean transformGroup]</h3>
<p>
This option only works if the [page:DragControls.objects] array contains a single draggable group object.
If set to `true`, [name] does not transform individual objects but the entire group. Default is `false`.
</p>

<h2>Methods</h2>

<p>See the base [page:EventDispatcher] class for common methods.</p>
<p>See the base [page:Controls] class for common methods.</p>

<h3>[method:undefined activate] ()</h3>
<h3>[method:undefined connect] ()</h3>
<p>
Adds the event listeners of the controls.
</p>

<h3>[method:undefined deactivate] ()</h3>
<h3>[method:undefined disconnect] ()</h3>
<p>
Removes the event listeners of the controls.
</p>
Expand All @@ -140,21 +142,6 @@ <h3>[method:undefined dispose] ()</h3>
Should be called if the controls is no longer required.
</p>

<h3>[method:Array getObjects] ()</h3>
<p>
Returns the array of draggable objects.
</p>

<h3>[method:Raycaster getRaycaster] ()</h3>
<p>
Returns the internal [page:Raycaster] instance that is used for intersection tests.
</p>

<h3>[method:undefined setObjects] ( [param:Array objects] )</h3>
<p>
Sets an array of draggable objects by overwriting the existing one.
</p>

<h2>Source</h2>

<p>
Expand Down
32 changes: 32 additions & 0 deletions examples/jsm/controls/Controls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { EventDispatcher } from 'three';

class Controls extends EventDispatcher {

constructor( object, domElement ) {
Copy link
Owner

Choose a reason for hiding this comment

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

@Mugen87

constructor( object, domElement = null ) maybe?

Right now it's undefined by default but subclasses set it to null...


super();

this.object = object;
this.domElement = domElement;

this.enabled = true;

this.state = - 1;

this.keys = {};
this.mouseButtons = { LEFT: null, MIDDLE: null, RIGHT: null };
this.touches = { ONE: null, TWO: null };

}

connect() {}
Copy link
Owner

Choose a reason for hiding this comment

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

@Mugen87 What do you think about being able to pass the dom element here?

That way the code could look like this:

const controls = new OrbitControls( camera );
controls.connect( renderer.domElement );

I guess we'll need to do a bit of management though:

connect( element ) {

    if ( this.domElement !== element ) {

        this.disconnect();

    }

    this.domElement = element;

}

Copy link
Collaborator Author

@Mugen87 Mugen87 Sep 9, 2024

Choose a reason for hiding this comment

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

As a convenient feature, I guess it's fine.

That said, domElement is public so devs can basically achieve the same without further changes to the control classes. However, the additional management code in connect() could prevent mistakes when devs want to exchange the DOM element (not sure how common this use case is though).


disconnect() {}

dispose() {}

update() {}

}

export { Controls };
Loading