Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 84cba492 authored by Michal Brzezinski's avatar Michal Brzezinski
Browse files

Fixing tiles scrolling with arrow keys 

Tiles were scrolling whenever ACTION_UP event was received on page indicator but that could also happen when user moves focus with arrows and only lands on page indicator.
Solution is to react on ACTION_UP event only after first (instead of repeated) ACTION_DOWN was received on this view as well and view doesn't lose focus in the meantime.
This logic is a bit more complicated so decided to make it internal in PageIndicator and extract this logic to separate class: LeftRightArrowPressedListener

Test: LeftRightArrowPressedListenerTest
Test: PagedTileLayoutTest
Test: navigate shade with arrows and see tiles don't scroll when focus lands on them
Fixes: 316554707
Flag: None

Change-Id: I59cfc851971b1e52f68c6a7279bd9bf1e62af159
parent 6f601837

packages/SystemUI/src/com/android/systemui/qs/LeftRightArrowPressedListener.kt

0 → 100644
+75 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2023 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.systemui.qs



import android.view.KeyEvent

import android.view.View

import androidx.core.util.Consumer



/**

 * Listens for left and right arrow keys pressed while focus is on the view.

 *

 * Key press is treated as correct when its full lifecycle happened on the view: first

 * [KeyEvent.ACTION_DOWN] was performed, view didn't lose focus in the meantime and then

 * [KeyEvent.ACTION_UP] was performed with the same [KeyEvent.getKeyCode]

 */

class LeftRightArrowPressedListener private constructor() :

    View.OnKeyListener, View.OnFocusChangeListener {



    private var lastKeyCode: Int? = 0

    private var listener: Consumer<Int>? = null



    fun setArrowKeyPressedListener(arrowPressedListener: Consumer<Int>) {

        listener = arrowPressedListener

    }



    override fun onKey(view: View, keyCode: Int, keyEvent: KeyEvent): Boolean {

        if (keyCode == KeyEvent.KEYCODE_DPAD_LEFT || keyCode == KeyEvent.KEYCODE_DPAD_RIGHT) {

            // only scroll on ACTION_UP as we don't handle longpressing for now. Still we need

            // to intercept even ACTION_DOWN otherwise keyboard focus will be moved before we

            // have a chance to intercept ACTION_UP.

            if (keyEvent.action == KeyEvent.ACTION_UP && keyCode == lastKeyCode) {

                listener?.accept(keyCode)

                lastKeyCode = null

            } else if (keyEvent.repeatCount == 0) {

                // we only read key events that are NOT coming from long pressing because that also

                // causes reading ACTION_DOWN event (with repeated count > 0) when moving focus with

                // arrow from another sibling view

                lastKeyCode = keyCode

            }

            return true

        }

        return false

    }



    override fun onFocusChange(view: View, hasFocus: Boolean) {

        // resetting lastKeyCode so we get fresh cleared state on focus

        if (hasFocus) {

            lastKeyCode = null

        }

    }



    companion object {

        @JvmStatic

        fun createAndRegisterListenerForView(view: View): LeftRightArrowPressedListener {

            val listener = LeftRightArrowPressedListener()

            view.setOnKeyListener(listener)

            view.onFocusChangeListener = listener

            return listener

        }

    }

}

packages/SystemUI/src/com/android/systemui/qs/PageIndicator.java

+29 −0
Original line number Diff line number Diff line 
package com.android.systemui.qs;



import static com.android.systemui.qs.PageIndicator.PageScrollActionListener.LEFT;

import static com.android.systemui.qs.PageIndicator.PageScrollActionListener.RIGHT;



import android.content.Context;

import android.content.res.ColorStateList;

import android.content.res.Resources;
@@ -9,10 +12,12 @@ import android.graphics.drawable.AnimatedVectorDrawable; 
import android.graphics.drawable.Drawable;

import android.util.AttributeSet;

import android.util.Log;

import android.view.KeyEvent;

import android.view.View;

import android.view.ViewGroup;

import android.widget.ImageView;



import androidx.annotation.IntDef;

import androidx.annotation.NonNull;



import com.android.settingslib.Utils;
@@ -43,6 +48,7 @@ public class PageIndicator extends ViewGroup { 


    private int mPosition = -1;

    private boolean mAnimating;

    private PageScrollActionListener mPageScrollActionListener;



    private final Animatable2.AnimationCallback mAnimationCallback =

            new Animatable2.AnimationCallback() {
@@ -77,6 +83,14 @@ public class PageIndicator extends ViewGroup { 
        mPageIndicatorWidth = res.getDimensionPixelSize(R.dimen.qs_page_indicator_width);

        mPageIndicatorHeight = res.getDimensionPixelSize(R.dimen.qs_page_indicator_height);

        mPageDotWidth = res.getDimensionPixelSize(R.dimen.qs_page_indicator_dot_width);

        LeftRightArrowPressedListener arrowListener =

                LeftRightArrowPressedListener.createAndRegisterListenerForView(this);

        arrowListener.setArrowKeyPressedListener(keyCode -> {

            if (mPageScrollActionListener != null) {

                int swipeDirection = keyCode == KeyEvent.KEYCODE_DPAD_LEFT ? LEFT : RIGHT;

                mPageScrollActionListener.onScrollActionTriggered(swipeDirection);

            }

        });

    }



    public void setNumPages(int numPages) {
@@ -280,4 +294,19 @@ public class PageIndicator extends ViewGroup { 
            getChildAt(i).layout(left, 0, mPageIndicatorWidth + left, mPageIndicatorHeight);

        }

    }



    void setPageScrollActionListener(PageScrollActionListener listener) {

        mPageScrollActionListener = listener;

    }



    interface PageScrollActionListener {



        @IntDef({LEFT, RIGHT})

        @interface Direction { }



        int LEFT = 0;

        int RIGHT = 1;



        void onScrollActionTriggered(@Direction int swipeDirection);

    }

}

packages/SystemUI/src/com/android/systemui/qs/PagedTileLayout.java

+10 −16
Original line number Diff line number Diff line 
package com.android.systemui.qs;



import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_SHADE_QS_SCROLL_SWIPE;

import static com.android.systemui.qs.PageIndicator.PageScrollActionListener.LEFT;

import static com.android.systemui.qs.PageIndicator.PageScrollActionListener.RIGHT;



import android.animation.Animator;

import android.animation.AnimatorListenerAdapter;
@@ -12,7 +14,6 @@ import android.content.Context; 
import android.content.res.Configuration;

import android.os.Bundle;

import android.util.AttributeSet;

import android.view.KeyEvent;

import android.view.LayoutInflater;

import android.view.View;

import android.view.ViewGroup;
@@ -30,6 +31,7 @@ import androidx.viewpager.widget.ViewPager; 
import com.android.internal.jank.InteractionJankMonitor;

import com.android.internal.logging.UiEventLogger;

import com.android.systemui.plugins.qs.QSTile;

import com.android.systemui.qs.PageIndicator.PageScrollActionListener.Direction;

import com.android.systemui.qs.QSPanel.QSTileLayout;

import com.android.systemui.qs.QSPanelControllerBase.TileRecord;

import com.android.systemui.qs.logging.QSLogger;
@@ -310,26 +312,18 @@ public class PagedTileLayout extends ViewPager implements QSTileLayout { 
        mPageIndicator = indicator;

        mPageIndicator.setNumPages(mPages.size());

        mPageIndicator.setLocation(mPageIndicatorPosition);

        mPageIndicator.setOnKeyListener((view, keyCode, keyEvent) -> {

            if (keyCode == KeyEvent.KEYCODE_DPAD_LEFT || keyCode == KeyEvent.KEYCODE_DPAD_RIGHT) {

                // only scroll on ACTION_UP as we don't handle longpressing for now. Still we need

                // to intercept even ACTION_DOWN otherwise keyboard focus will be moved before we

                // have a chance to intercept ACTION_UP.

                if (keyEvent.getAction() == KeyEvent.ACTION_UP && mScroller.isFinished()) {

                    scrollByX(getDeltaXForKeyboardScrolling(keyCode),

        mPageIndicator.setPageScrollActionListener(swipeDirection -> {

            if (mScroller.isFinished()) {

                scrollByX(getDeltaXForPageScrolling(swipeDirection),

                        SINGLE_PAGE_SCROLL_DURATION_MILLIS);

            }

                return true;

            }

            return false;

        });

    }



    private int getDeltaXForKeyboardScrolling(int keyCode) {

        if (keyCode == KeyEvent.KEYCODE_DPAD_LEFT && getCurrentItem() != 0) {

    private int getDeltaXForPageScrolling(@Direction int swipeDirection) {

        if (swipeDirection == LEFT && getCurrentItem() != 0) {

            return -getWidth();

        } else if (keyCode == KeyEvent.KEYCODE_DPAD_RIGHT

                && getCurrentItem() != mPages.size() - 1) {

        } else if (swipeDirection == RIGHT && getCurrentItem() != mPages.size() - 1) {

            return getWidth();

        }

        return 0;

packages/SystemUI/tests/src/com/android/systemui/qs/LeftRightArrowPressedListenerTest.kt

0 → 100644
+107 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2023 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.systemui.qs



import android.testing.AndroidTestingRunner

import android.view.KeyEvent

import android.view.KeyEvent.KEYCODE_DPAD_LEFT

import android.view.View

import androidx.core.util.Consumer

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase

import com.google.common.truth.Truth.assertThat

import org.junit.Before

import org.junit.Test

import org.junit.runner.RunWith



@RunWith(AndroidTestingRunner::class)

@SmallTest

class LeftRightArrowPressedListenerTest : SysuiTestCase() {



    private lateinit var underTest: LeftRightArrowPressedListener

    private val callback =

        object : Consumer<Int> {

            var lastValue: Int? = null



            override fun accept(keyCode: Int?) {

                lastValue = keyCode

            }

        }



    private val view = View(context)



    @Before

    fun setUp() {

        underTest = LeftRightArrowPressedListener.createAndRegisterListenerForView(view)

        underTest.setArrowKeyPressedListener(callback)

    }



    @Test

    fun shouldTriggerCallback_whenArrowUpReceived_afterArrowDownReceived() {

        underTest.sendKey(KeyEvent.ACTION_DOWN, KEYCODE_DPAD_LEFT)



        underTest.sendKey(KeyEvent.ACTION_UP, KEYCODE_DPAD_LEFT)



        assertThat(callback.lastValue).isEqualTo(KEYCODE_DPAD_LEFT)

    }



    @Test

    fun shouldNotTriggerCallback_whenKeyUpReceived_ifKeyDownNotReceived() {

        underTest.sendKey(KeyEvent.ACTION_UP, KEYCODE_DPAD_LEFT)



        assertThat(callback.lastValue).isNull()

    }



    @Test

    fun shouldNotTriggerCallback_whenKeyUpReceived_ifKeyDownWasRepeated() {

        underTest.sendKeyWithRepeat(KeyEvent.ACTION_UP, KEYCODE_DPAD_LEFT, repeat = 2)

        underTest.sendKey(KeyEvent.ACTION_UP, KEYCODE_DPAD_LEFT)



        assertThat(callback.lastValue).isNull()

    }



    @Test

    fun shouldNotTriggerCallback_whenKeyUpReceived_ifKeyDownReceivedBeforeLosingFocus() {

        underTest.sendKey(KeyEvent.ACTION_DOWN, KEYCODE_DPAD_LEFT)

        underTest.onFocusChange(view, hasFocus = false)

        underTest.onFocusChange(view, hasFocus = true)



        underTest.sendKey(KeyEvent.ACTION_UP, KEYCODE_DPAD_LEFT)



        assertThat(callback.lastValue).isNull()

    }



    private fun LeftRightArrowPressedListener.sendKey(action: Int, keyCode: Int) {

        onKey(view, keyCode, KeyEvent(action, keyCode))

    }



    private fun LeftRightArrowPressedListener.sendKeyWithRepeat(

        action: Int,

        keyCode: Int,

        repeat: Int

    ) {

        val keyEvent =

            KeyEvent(

                /* downTime= */ 0L,

                /* eventTime= */ 0L,

                /* action= */ action,

                /* code= */ KEYCODE_DPAD_LEFT,

                /* repeat= */ repeat

            )

        onKey(view, keyCode, keyEvent)

    }

}

packages/SystemUI/tests/src/com/android/systemui/qs/PagedTileLayoutTest.kt

+11 −10
Original line number Diff line number Diff line
@@ -2,11 +2,13 @@ package com.android.systemui.qs 


import android.content.Context

import android.testing.AndroidTestingRunner

import android.view.KeyEvent

import android.view.View

import android.widget.Scroller

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase

import com.android.systemui.qs.PageIndicator.PageScrollActionListener

import com.android.systemui.qs.PageIndicator.PageScrollActionListener.LEFT

import com.android.systemui.qs.PageIndicator.PageScrollActionListener.RIGHT

import com.google.common.truth.Truth.assertThat

import org.junit.Before

import org.junit.Test
@@ -22,7 +24,7 @@ import org.mockito.MockitoAnnotations 
class PagedTileLayoutTest : SysuiTestCase() {



    @Mock private lateinit var pageIndicator: PageIndicator

    @Captor private lateinit var captor: ArgumentCaptor<View.OnKeyListener>

    @Captor private lateinit var captor: ArgumentCaptor<PageScrollActionListener>



    private lateinit var pageTileLayout: TestPagedTileLayout

    private lateinit var scroller: Scroller
@@ -32,7 +34,7 @@ class PagedTileLayoutTest : SysuiTestCase() { 
        MockitoAnnotations.initMocks(this)

        pageTileLayout = TestPagedTileLayout(mContext)

        pageTileLayout.setPageIndicator(pageIndicator)

        verify(pageIndicator).setOnKeyListener(captor.capture())

        verify(pageIndicator).setPageScrollActionListener(captor.capture())

        setViewWidth(pageTileLayout, width = PAGE_WIDTH)

        scroller = pageTileLayout.mScroller

    }
@@ -43,28 +45,27 @@ class PagedTileLayoutTest : SysuiTestCase() { 
    }



    @Test

    fun scrollsRight_afterRightArrowPressed_whenFocusOnPagerIndicator() {

    fun scrollsRight_afterRightScrollActionTriggered() {

        pageTileLayout.currentPageIndex = 0



        sendUpEvent(KeyEvent.KEYCODE_DPAD_RIGHT)

        sendScrollActionEvent(RIGHT)



        assertThat(scroller.isFinished).isFalse() // aka we're scrolling

        assertThat(scroller.finalX).isEqualTo(scroller.currX + PAGE_WIDTH)

    }



    @Test

    fun scrollsLeft_afterLeftArrowPressed_whenFocusOnPagerIndicator() {

    fun scrollsLeft_afterLeftScrollActionTriggered() {

        pageTileLayout.currentPageIndex = 1 // we won't scroll left if we're on the first page



        sendUpEvent(KeyEvent.KEYCODE_DPAD_LEFT)

        sendScrollActionEvent(LEFT)



        assertThat(scroller.isFinished).isFalse() // aka we're scrolling

        assertThat(scroller.finalX).isEqualTo(scroller.currX - PAGE_WIDTH)

    }



    private fun sendUpEvent(keyCode: Int) {

        val event = KeyEvent(KeyEvent.ACTION_UP, keyCode)

        captor.value.onKey(pageIndicator, keyCode, event)

    private fun sendScrollActionEvent(@PageScrollActionListener.Direction direction: Int) {

        captor.value.onScrollActionTriggered(direction)

    }



    /**