Skip to content

feat: add hover effects to divider in ResizablePaneManager #18952

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

Merged
merged 2 commits into from
Jul 26, 2025

Conversation

Haz3-jolt
Copy link
Contributor

Purpose / Description

  • Resizable divider onHover changes PointerIcon to a resizing indicator and sets divider to dragColor

Fixes

  • For GSoC 2025: Tablet & Chromebook UI

Approach

  • As suggested by @SanjaySargam this will add a onHoverListner to the ResizablePaneManager to show users that it is resizable

How Has This Been Tested?

Tested on Chromebook emulator - API 34
hover.webm

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@Haz3-jolt Haz3-jolt added Needs Review GSoC Pull requests authored by a Google Summer of Code participant [Candidate/Selected], for GSoC mentors labels Jul 23, 2025
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

Could you make the color change much more subtle (probably to another gray)

It's a jarring color transition right now for a hover effect

@Haz3-jolt
Copy link
Contributor Author

Could you make the color change much more subtle (probably to another gray)

It's a jarring color transition right now for a hover effect

I chose this because it's the norm for resizing screens on desktop sites:

colors.mp4

@david-allison
Copy link
Member

david-allison commented Jul 24, 2025

Ok, the video works.

Have the area which changes color match the width of the area in the video you provided.

true
}
MotionEvent.ACTION_HOVER_EXIT -> {
divider.setBackgroundColor(idleColor)
Copy link
Member

Choose a reason for hiding this comment

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

duplicates some color-setting behavior from touch events - consider extracting common visual state methods

Something like this:

private fun setDividerVisualState(view: View, backgroundColor: Int, pointerIcon: PointerIcon?) {
              view.setBackgroundColor(backgroundColor)
              view.pointerIcon = pointerIcon
      }

@Haz3-jolt
Copy link
Contributor Author

Ok, the video works.

Have the area which changes color match the width of the area in the video you provided.

Wait so I should reduce the width of the divider? because all it is doing is changing it's background color.

@david-allison
Copy link
Member

  • add an issue so we can deal with this later, no hover colour is better than a bad hoover colour
  • Keep the pointer effect, we can get this PR in now
  • Consider moving the Frame Layout + View to a custom view, so we can better handle onDraw

@Haz3-jolt
Copy link
Contributor Author

  • add an issue so we can deal with this later, no hover colour is better than a bad hoover colour

    • Keep the pointer effect, we can get this PR in now

    • Consider moving the Frame Layout + View to a custom view, so we can better handle onDraw

I'll update it tomorrow!

@Haz3-jolt Haz3-jolt force-pushed the Hover branch 2 times, most recently from 5ae9860 to 31d62fe Compare July 25, 2025 11:29
@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Jul 25, 2025
Copy link
Member

@SanjaySargam SanjaySargam left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@Haz3-jolt Haz3-jolt added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Jul 26, 2025
@david-allison david-allison added this pull request to the merge queue Jul 26, 2025
Merged via the queue into ankidroid:main with commit 06ae615 Jul 26, 2025
10 checks passed
@github-actions github-actions bot added this to the 2.22 release milestone Jul 26, 2025
@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Jul 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC Pull requests authored by a Google Summer of Code participant [Candidate/Selected], for GSoC mentors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants