Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a move bar widget #1627

Merged
merged 2 commits into from
Dec 3, 2024
Merged

Add a move bar widget #1627

merged 2 commits into from
Dec 3, 2024

Conversation

svillar
Copy link
Member

@svillar svillar commented Nov 12, 2024

This widget replaces the experimental move windows button that could be enabled from the Display Settings. That button was shown in the navigation bar and was a bit weird to use.

This PR replaces it with a thing semi-transparent bar that when dragged moves the window in the horizontal direction as the old button used to do. That bar is unconditionally shown when in curved mode.

@svillar
Copy link
Member Author

svillar commented Nov 12, 2024

For reference, this is how the bar looks like
com oculus vrshell-20241112-110737
and this is its appearance when hovered
com oculus vrshell-20241112-110808

@svillar
Copy link
Member Author

svillar commented Nov 14, 2024

I fixed an error in the XML file that was preventing the navigation url bar being shown while in not curved mode

@felipeerias
Copy link
Collaborator

I think that the move bar is still visible in resize mode, but it can not be used. It should probably be hidden in that case.

@svillar
Copy link
Member Author

svillar commented Nov 14, 2024

I think that the move bar is still visible in resize mode, but it can not be used. It should probably be hidden in that case.

I have tried and it is not visible, so working as expected.

That said I found a weird bug. When putting a standard YT video in fullscreen mode and then unfullscreening it, the move bar dissapears from the front window. It's shown in the others but not in the front one. I'm checking.

@svillar
Copy link
Member Author

svillar commented Nov 14, 2024

I think that the move bar is still visible in resize mode, but it can not be used. It should probably be hidden in that case.

I have tried and it is not visible, so working as expected.

That said I found a weird bug. When putting a standard YT video in fullscreen mode and then unfullscreening it, the move bar dissapears from the front window. It's shown in the others but not in the front one. I'm checking.

The bug was not caused by the patch, but unveiled instead. I've included a commit that fixes it.

@svillar svillar requested a review from felipeerias November 14, 2024 18:06
@svillar svillar marked this pull request as draft November 15, 2024 16:40
@svillar svillar requested a review from felipeerias November 28, 2024 12:13
@svillar svillar marked this pull request as ready for review November 28, 2024 12:13
@svillar
Copy link
Member Author

svillar commented Nov 28, 2024

@felipeerias it'd be nice if you could take another look to this

@felipeerias
Copy link
Collaborator

@svillar I think that this is almost ready, great job!

One issue is that the alpha of the view is set to MIN when the user clicks on it. I think that this might be caused by some interaction between the hover and touch events.

How about keeping alpha the same and changing the background of the move bar in response to different events?

Something like this:

ScreenRecording_2024.11.29-12.26.04.mp4
  • navigation_bar_navigation.xml
            <View
                android:layout_width="@dimen/move_bar_width"
                android:layout_height="8dp"
                android:layout_gravity="center"
                android:background="@drawable/move_bar_background"
                android:duplicateParentState="true"
                android:radius="4dp" />
  • move_bar_background.xml
<selector xmlns:android="http://schemas.android.com/apk/res/android">
    <item android:state_pressed="true">
        <shape>
            <solid android:color="@color/ocean" />
            <corners android:radius="5dp" />
            <size android:height="5dp" />
            <!-- 50% opacity @color/asphalt -->
            <stroke android:width="1dp" android:color="#3d3d3d80" />
        </shape>
    </item>
    <item android:state_hovered="true">
        <shape>
            <solid android:color="@color/azure" />
            <corners android:radius="5dp" />
            <size android:height="5dp" />
            <stroke android:width="1dp" android:color="#3d3d3d80" />
        </shape>
    </item>
    <item>
        <shape>
            <solid android:color="@color/fog" />
            <corners android:radius="5dp" />
            <size android:height="5dp" />
            <stroke android:width="1dp" android:color="#3d3d3d80" />
        </shape>
    </item>
</selector>

The colors are just an example, but I think that it would be a good idea to use a similar color scheme to #1647 for consistency.

@svillar
Copy link
Member Author

svillar commented Nov 29, 2024

@svillar I think that this is almost ready, great job!

One issue is that the alpha of the view is set to MIN when the user clicks on it. I think that this might be caused by some interaction between the hover and touch events.

Yes you're right. The problem is that we receive the hover exit event just before the touch down event is emitted. We cannot know at that point whether the hover exit was emitted because the pointer moved away or because a touch down event is about to arrive.

How about keeping alpha the same and changing the background of the move bar in response to different events?

The colors are just an example, but I think that it would be a good idea to use a similar color scheme to #1647 for consistency.

The video looks good, I'll follow that suggestion.

This widget replaces the experimental move windows button that could
be enabled from the Display Settings. That button was shown in the
navigation bar and was a bit weird to use.

This PR replaces it with a thing semi-transparent bar that when
dragged moves the window in the horizontal direction as the old
button used to do. That bar is unconditionally shown when in
curved mode.
Copy link
Collaborator

@felipeerias felipeerias left a comment

Choose a reason for hiding this comment

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

Very good, LGTM. I've added a couple small notes but otherwise I think we can merge this.

Window view model's isCurved property was not properly updated when
exiting fullscreen. When entering fullscreen it's updated because
placeWindow() is called. However on unfullscreen placeWindow() is
not called so the isCurved property was not updated. The solution
is to call updateCurvedMode when entering/exiting fullscreen.
@svillar svillar merged commit 1890d23 into main Dec 3, 2024
22 checks passed
@svillar svillar deleted the moving_bar branch December 3, 2024 15:55
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.

2 participants