Skip to content

feat: duration picker updated with mouse wheel#1157

Open
MitanOmar wants to merge 1 commit intoadfinis:mainfrom
MitanOmar:duration-picker-updated-with-mouse-wheel
Open

feat: duration picker updated with mouse wheel#1157
MitanOmar wants to merge 1 commit intoadfinis:mainfrom
MitanOmar:duration-picker-updated-with-mouse-wheel

Conversation

@MitanOmar
Copy link
Copy Markdown
Member

fix: #36

@MitanOmar MitanOmar self-assigned this Apr 16, 2026
@MitanOmar MitanOmar marked this pull request as ready for review April 16, 2026 06:24
@MitanOmar MitanOmar requested a review from a team as a code owner April 16, 2026 06:24
Copy link
Copy Markdown
Member

@derrabauke derrabauke left a comment

Choose a reason for hiding this comment

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

Nitpick: could you correct the commit message to something which reads like a sentence?

e.g. "feat(duration-picker): adds mouse wheel listener"

gracias :-)

Comment on lines +31 to +39
@action
wheel(event) {
debounceTask(this, "wheelAction", event.deltaY, 100);
}

wheelAction(deltaY) {
const direction = deltaY > 0 ? -1 : 1;
this._addMinutes(this.precision * direction);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
@action
wheel(event) {
debounceTask(this, "wheelAction", event.deltaY, 100);
}
wheelAction(deltaY) {
const direction = deltaY > 0 ? -1 : 1;
this._addMinutes(this.precision * direction);
}
@action
wheelAction(event) {
debounceTask(this, "wheelAction", event.deltaY, 100);
}
#wheel(deltaY) {
const direction = deltaY > 0 ? -1 : 1;
this._addMinutes(this.precision * direction);
}

Clarify naming.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

implemented

it's happen that my dev tools was open, and the page height was more then the window height,
so it was scrolling the page and updating the value.

so by preventing the default, the window scroll will stop if the mouse on duration input

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

# missing. Either that or _wheel to show that this is private API.

@MitanOmar MitanOmar force-pushed the duration-picker-updated-with-mouse-wheel branch from b7ca497 to d7d9e8c Compare April 16, 2026 08:13
@MitanOmar MitanOmar requested a review from derrabauke April 16, 2026 08:13
@MitanOmar MitanOmar force-pushed the duration-picker-updated-with-mouse-wheel branch from d7d9e8c to d7c5969 Compare April 16, 2026 08:33
@derrabauke
Copy link
Copy Markdown
Member

derrabauke commented Apr 16, 2026

Just tested it locally. The scroll to increase doesn't feel good. It only increases by 15 even if I scroll really slow, since it's debounced. It would be better to compare the event.deltaY/scrollTop and increase after a certain distance threshold. Using debounceTask throttleTask improves it already massively.

@MitanOmar
Copy link
Copy Markdown
Member Author

@derrabauke i tried that, but then it will go from 15 to 45 directly, and the user may confuse with the scroll amount and speed.

is that OK ?

@MitanOmar MitanOmar force-pushed the duration-picker-updated-with-mouse-wheel branch from d7c5969 to 869c320 Compare April 16, 2026 08:49
Comment on lines +31 to +35
@action
wheelAction(event) {
event.preventDefault();
debounceTask(this, "_wheel", event.deltaY, 100);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
@action
wheelAction(event) {
event.preventDefault();
debounceTask(this, "_wheel", event.deltaY, 100);
}
@action
wheelAction(event) {
event.preventDefault();
this.delta += event.wheelDeltaY;
if (this.delta > 50 || this.delta < -50) {
this.wheel(this.delta)
this.delta = 0;
}
}

This makes it snappy fast and throttles the scrolling so it's controllable even with the trackpad.

@@ -1,4 +1,5 @@
import { action } from "@ember/object";
import { debounceTask } from "ember-lifeline";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not necessary.

@c0rydoras
Copy link
Copy Markdown
Collaborator

Or, we use the existing durationpicker in the ui-core addon that has this already?

https://github.com/adfinis/timed/blob/main/ember/ui-core/src/components/durationpicker.gts#L114-L120

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.

[FEATURE]: durationpicker scroll -> +/- 15 Minuten

3 participants