Merge lp://qastaging/~gandalfn/slingshot/fix-input-event into lp://qastaging/~elementary-pantheon/slingshot/trunk

Proposed by Zisu Andrei
Status: Rejected
Rejected by: Cody Garver
Proposed branch: lp://qastaging/~gandalfn/slingshot/fix-input-event
Merge into: lp://qastaging/~elementary-pantheon/slingshot/trunk
Diff against target: 111 lines (+24/-11) (has conflicts)
1 file modified
src/SlingshotView.vala (+24/-11)
Text conflict in src/SlingshotView.vala
To merge this branch: bzr merge lp://qastaging/~gandalfn/slingshot/fix-input-event
Reviewer Review Type Date Requested Status
Cody Garver (community) Disapprove
Adam Bieńkowski (community) code Needs Fixing
Review via email: mp+312215@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
quequotion (quequotion) wrote :

Are these supposed to be here?

<<<<<<< TREE
=======
>>>>>>> MERGE-SOURCE

Revision history for this message
Adam Bieńkowski (donadigo) wrote :

Conflicts with trunk.

review: Needs Fixing (code)
Revision history for this message
quequotion (quequotion) wrote :

> Are these supposed to be here?
>
> <<<<<<< TREE
> =======
> >>>>>>> MERGE-SOURCE

After fixing that part of the patch, gandalfn's merge proposal works.

This patch is very similar to the pantheon-debian patch:
https://gitlab.com/pantheon-debian/slingshot-launcher/commit/db8474f079a02ad005d662040ff6452c1e2f0b05

Except for this section:
@@ -402,8 +408,10 @@
                 case "9":
                     int page = int.parse (key);

- if (event.state != Gdk.ModifierType.MOD1_MASK)
- return false;
+ if (event.state != Gdk.ModifierType.MOD1_MASK) {
+ search_entry.key_press_event (event);
+ return true;
+ }

                     if (modality == Modality.NORMAL_VIEW) {
                         if (page < 0 || page == 9)

Is either preferrable for any reason?

Revision history for this message
Zisu Andrei (matzipan) wrote :

This seems to be an inactive branch. Perhaps we should create a new merge request based on this.

Revision history for this message
quequotion (quequotion) wrote :

>>matzipan
I've updated my branch, rebased upstream, and reimplemented gandalfn's method:
https://code.launchpad.net/~quequotion/slingshot/fix-1635776/+merge/310705

Revision history for this message
Cody Garver (codygarver) wrote :

I am rejecting this branch because we are accepting the simpler proposal (https://code.launchpad.net/~santileortiz/slingshot/fix-1635776/+merge/314255), which removes old hacky code that appears to be the source of the problem.

However, we've already reverted 1 commit that intended to fix this problem, so please stick around to make sure the problem is finally solved.

review: Disapprove

Unmerged revisions

651. By gandalfn

The scroll and key press/release event forwading from event box does not
work on debian testing.
Activate the default event mask for this events for event box.
Disconnect search entry key press/release event from event forwarding.
Send key press/release event forwarding from event box to search entry in all case.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches