feat: Add shape option to joystick

[htshah]

Feature add for issue #104.

[yoannmoinet]

That's an awesome new feature, thanks a lot.

Can you clean your PR though?
There are a lot of changes that are not part of the feature.

The package-lock.json shouldn't change, neither should the code style.

NOTE: You can change the code style, but it should be done in another PR and enforced by something automatic. Here, it seems to be just a local configuration of yours. Feel free to add any linter, as long as it's automatically enforced.

[htshah]

Changes done:

• Reverted package-lock.json
• Reverted all unrelated style changes

[Nek-]

Hello guys. Any update on this? Should this work be finish? (it seems that only a rebase is required)

[htshah]

@yoannmoinet
Just a follow up for this PR, is there anything else on which you would like to comment?

[yoannmoinet]

I'm off with no computer.
It will have to wait next week end.
I'll add a reminder.

[yoannmoinet]

Thank you for taking the time to implement this.
I have a few suggestions though.
Also, you're missing the types, you need to update ./types/index.d.ts to add the new option.

[yoannmoinet]

No need to set it up in case it's a square.
Simply put it in a condition and get rid of this dual variable declaration.

Suggested change

	u.extend(
	styles.back,
	this.options.shape === 'circle' ? circleBorderStyle : squareBorderStyle
	);
	if (this.options.shape === 'circle') {
	u.extend(styles.back, borderStyle);
	}

[htshah]

I've made the necessary changes for this suggestion!

[yoannmoinet]

Not a fan of switch.
I'd prefer a simple if - else statement.
Starting with the default behavior, aka if (nipple.options.shape === 'circle') {

[yoannmoinet]

Although I think this process should be simplified.
Something like:

/* utils.js */
export const clamp = (pos, min, max) => ({
    x: Math.min(Math.max(pos.x, min), max),
    y: Math.min(Math.max(pos.y, min), max)
});

/* collection.js */

// Clamp the position.
if (nipple.options.shape === 'circle') {
    dist = Math.min(dist, size);
    pos = u.findCoord(nipple.position, dist, angle);
} else {
    // Clamp to a square
    pos = u.clamp(nipple.position, -size, size);
    dist = u.distance(pos, nipple.position);
}

var xPosition = pos.x - nipple.position.x;
var yPosition = pos.y - nipple.position.y;

What do you think?

[htshah]

@yoannmoinet
Thank you for this great suggestion! I've implemented it but had to make some minor changes to the suggested clamp() function. Here's the updated function:

/* utils.js */
// Clamp position within the range
export const clamp = (pos, nipplePos, size) => ({
    //                          left-clamping        right-clamping
    x: Math.min(Math.max(pos.x, nipplePos.x - size), nipplePos.x + size),
    //                          top-clamping         bottom-clamping
    y: Math.min(Math.max(pos.y, nipplePos.y - size), nipplePos.y + size)
});

Also, I've replaced switch statements with if-else statements as you requested.
Do let me know if there are any further changes required!

Thanks!

[yoannmoinet]

Well done, way simpler now.
LGTM.

[yoannmoinet]

There are conflicts.
Once they are fixed I'll merge the PR.

[yoannmoinet]

NVM, I'll squash and merge.

[yoannmoinet]

Published in v0.8.6

[htshah]

Thank you soo much for accepting this PR. It is my first ever open source contribution.😁

[yoannmoinet]

No problem @htshah that was a good contribution.
Well done, and thank YOU!