Issue 302700043: ui/gtk3: support scroll event in candidates panel (Closed)
Descriptionui/gtk3: support scroll event in candidates panel
When press scroll button of mouse on candidates,
automatically cursor up/cursor down.
BUG=
Patch Set 1 #
Total comments: 1
Patch Set 2 : ui/gtk3: support scroll event in candidates panel #
Total comments: 8
Patch Set 3 : ui/gtk3: support scroll event in candidates panel #Patch Set 4 : ui/gtk3: support scroll event in candidates panel #Patch Set 5 : ui/gtk3: support scroll event in candidates panel #
Total comments: 1
Patch Set 6 : ui/gtk3: support scroll event in candidates panel #MessagesTotal messages: 20
I tested Peng Wu’s patch on Fedora 24 and it works for me.
Sign in to reply to this message.
I think scroll up/down is cursor up/down but not page up/down?
Sign in to reply to this message.
Yes, cursor up/down would be nicer than page up/down.
Sign in to reply to this message.
On 2016/09/14 09:27:55, maiku.fabian wrote: > Yes, cursor up/down would be nicer than page up/down. But page up/down is better than nothing.
Sign in to reply to this message.
https://codereview.appspot.com/302700043/diff/1/ui/gtk3/candidatepanel.vala File ui/gtk3/candidatepanel.vala (right): https://codereview.appspot.com/302700043/diff/1/ui/gtk3/candidatepanel.vala#n... ui/gtk3/candidatepanel.vala:266: page_up(); I think scroll up/down is cursor up/down but not page up/down?
Sign in to reply to this message.
I tested the patch after changing page_up()/page_down() to
cursor_up()/cursor_down(). Like this:
diff --git a/ui/gtk3/candidatepanel.vala b/ui/gtk3/candidatepanel.vala
index 3e3c1b5..b778969 100644
--- a/ui/gtk3/candidatepanel.vala
+++ b/ui/gtk3/candidatepanel.vala
@@ -262,10 +262,10 @@ public class CandidatePanel : Gtk.Box{
m_candidate_area.scroll_clicked.connect((direction) => {
switch (direction) {
case Gdk.ScrollDirection.UP:
- page_up();
+ cursor_up();
break;
case Gdk.ScrollDirection.DOWN:
- page_down();
+ cursor_down();
break;
}
return;
Unfortunately it doesn’t work anymore then.
I tried in ibus-anthy (which is implemented in python
and implements do_cursor_up() and do_cursor_down()).
And I also tried in ibus-typing-booster. ibus-typing-booster
did not yet implement do_cursor_up() and do_cursor_down(),
but adding these functions to ibus-typing-booster did not make it work.
Sign in to reply to this message.
On 2016/09/15 06:30:59, maiku.fabian wrote:
> I tested the patch after changing page_up()/page_down() to
> cursor_up()/cursor_down(). Like this:
>
> diff --git a/ui/gtk3/candidatepanel.vala b/ui/gtk3/candidatepanel.vala
> index 3e3c1b5..b778969 100644
> --- a/ui/gtk3/candidatepanel.vala
> +++ b/ui/gtk3/candidatepanel.vala
> @@ -262,10 +262,10 @@ public class CandidatePanel : Gtk.Box{
> m_candidate_area.scroll_clicked.connect((direction) => {
> switch (direction) {
> case Gdk.ScrollDirection.UP:
> - page_up();
> + cursor_up();
> break;
> case Gdk.ScrollDirection.DOWN:
> - page_down();
> + cursor_down();
> break;
> }
> return;
>
> Unfortunately it doesn’t work anymore then.
>
> I tried in ibus-anthy (which is implemented in python
> and implements do_cursor_up() and do_cursor_down()).
>
> And I also tried in ibus-typing-booster. ibus-typing-booster
> did not yet implement do_cursor_up() and do_cursor_down(),
> but adding these functions to ibus-typing-booster did not make it work.
I'd ask the patch owner why it does not work.
Sign in to reply to this message.
Okay, updated the patch to use cursor up/cursor down now.
Sign in to reply to this message.
On 2016/09/18 08:54:48, Peng Wu wrote: > Okay, updated the patch to use cursor up/cursor down now. I tested this and it works fine!
Sign in to reply to this message.
https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatearea.vala File ui/gtk3/candidatearea.vala (right): https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatearea.val... ui/gtk3/candidatearea.vala:54: public signal void scroll_clicked(Gdk.ScrollDirection direction); changed to candidate_scrolled? https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatearea.val... ui/gtk3/candidatearea.vala:224: label_ebox.scroll_event.connect((w, e) => { Can we add scroll_event handler on container of label_eboxes? https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatepanel.vala File ui/gtk3/candidatepanel.vala (right): https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatepanel.va... ui/gtk3/candidatepanel.vala:266: cursor_up(); why not emit cursor_{up,down} from candidate_area class directly?
Sign in to reply to this message.
https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatearea.vala File ui/gtk3/candidatearea.vala (right): https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatearea.val... ui/gtk3/candidatearea.vala:54: public signal void scroll_clicked(Gdk.ScrollDirection direction); On 2016/09/23 13:33:36, Peng wrote: > changed to candidate_scrolled? Done. https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatearea.val... ui/gtk3/candidatearea.vala:224: label_ebox.scroll_event.connect((w, e) => { On 2016/09/23 13:33:36, Peng wrote: > Can we add scroll_event handler on container of label_eboxes? If we add scroll_event handler on container of label_eboxes, other widgets will capture the scroll events, the container doesn't receive the scroll event, BICBW. https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatepanel.vala File ui/gtk3/candidatepanel.vala (right): https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatepanel.va... ui/gtk3/candidatepanel.vala:266: cursor_up(); On 2016/09/23 13:33:36, Peng wrote: > why not emit cursor_{up,down} from candidate_area class directly? I just want to keep the logic of cursor up/cursor down in one place, not in every scroll_event handler.
Sign in to reply to this message.
https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatearea.vala File ui/gtk3/candidatearea.vala (right): https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatearea.val... ui/gtk3/candidatearea.vala:224: label_ebox.scroll_event.connect((w, e) => { On 2016/09/26 08:20:33, Peng Wu wrote: > On 2016/09/23 13:33:36, Peng wrote: > > Can we add scroll_event handler on container of label_eboxes? > > If we add scroll_event handler on container of label_eboxes, > other widgets will capture the scroll events, > the container doesn't receive the scroll event, BICBW. > what are other widgets? Can we ignore scroll events for those widgets? https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatepanel.vala File ui/gtk3/candidatepanel.vala (right): https://codereview.appspot.com/302700043/diff/20001/ui/gtk3/candidatepanel.va... ui/gtk3/candidatepanel.vala:266: cursor_up(); On 2016/09/26 08:20:33, Peng Wu wrote: > On 2016/09/23 13:33:36, Peng wrote: > > why not emit cursor_{up,down} from candidate_area class directly? > > I just want to keep the logic of cursor up/cursor down in one place, > not in every scroll_event handler. I prefer to move this logic to CandidateArea class. This class doesn't need to know the scroll events.
Sign in to reply to this message.
I mean the label and candidates widgets on top of the container.
Sign in to reply to this message.
On 2016/09/27 08:20:47, Peng Wu wrote: > I mean the label and candidates widgets on top of the container. So can we ignore scroll events for label and candidates widgets, and handle it on a top level container? Please try it. thanks.
Sign in to reply to this message.
On 2016/09/27 14:38:45, Peng wrote: > So can we ignore scroll events for label and candidates widgets, and handle it > on a top level container? Please try it. thanks. Thanks for the comment! Done.
Sign in to reply to this message.
lgtm https://codereview.appspot.com/302700043/diff/100001/ui/gtk3/candidatearea.vala File ui/gtk3/candidatearea.vala (right): https://codereview.appspot.com/302700043/diff/100001/ui/gtk3/candidatearea.va... ui/gtk3/candidatearea.vala:242: This line is no need.
Sign in to reply to this message.
On 2016/10/11 05:30:10, fujiwara wrote: > lgtm > > https://codereview.appspot.com/302700043/diff/100001/ui/gtk3/candidatearea.vala > File ui/gtk3/candidatearea.vala (right): > > https://codereview.appspot.com/302700043/diff/100001/ui/gtk3/candidatearea.va... > ui/gtk3/candidatearea.vala:242: > This line is no need. lgtm
Sign in to reply to this message.
|
