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

Sidebar "on mouseover" transition should be smoother #21992

Closed
Brave-Matt opened this issue Mar 30, 2022 · 4 comments · Fixed by brave/brave-core#19080
Closed

Sidebar "on mouseover" transition should be smoother #21992

Brave-Matt opened this issue Mar 30, 2022 · 4 comments · Fixed by brave/brave-core#19080
Assignees
Labels
feature/sidebar Relating to Brave's Sidebar feature feature/user-interface All UI related OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Win64 QA/Yes release-notes/include

Comments

@Brave-Matt
Copy link

Description

When side bar is enabled with On mouseover selected, the Sidebar "animation" is rather jerky and just sort of appears/disappears when the cursor is positioned on the left side of the screen:

Screen.Recording.2022-03-30.at.1.02.22.PM.mov

While this is not a massive deal, I do think the feature could look/feel cleaner and more polished if we were to add a transition time when showing the Sidebar to give it a "smoother" feel.

Some examples:

@Brave-Matt Brave-Matt added OS/Desktop feature/sidebar Relating to Brave's Sidebar feature labels Mar 30, 2022
@NumDeP
Copy link

NumDeP commented Apr 29, 2022

Yeah I agree. I just used it for the first time on Youtube and the jittering of certain components on the site makes your eyes feel funny.

Update: @Brave-Matt this maybe be a dupe #21920

@rainer2208
Copy link

rainer2208 commented May 23, 2022

Hi, yes I also feel that the sidebar isn't as smooth as it could be.

When it appears, it just pushes the website to the right., what does not feel good. Instead it could just slide over the site. The experience would be a lot smoother.

@Brave-Matt
Copy link
Author

Bumping this thread since more features are being added to the sidebar (such as reading list). This is obviously not a browser breaking issue, but I think its worth further polishing/refining features like.

In the attached example, clicking the reading list on pops out right away (still jerky, as is in the original issue here), but then closes wonky — separately almost from the sidebar itself:

Screen.Recording.2022-08-24.at.10.07.27.AM.mov

simonhong added a commit to brave/brave-core that referenced this issue Jul 4, 2023
fix brave/brave-browser#21992
fix brave/brave-browser#25382

Added slide animation for opening/closing sidebar/side panel.
To use slide animation, this PR has some more refactoring/cleanup.

1. Ask to show/hide sidebar/panel when only needed.
2. Use SidePanelUI(SidePanelCoordinator) for starting side panel activating.
   So far, we use both(SidePanelUI/SidebarController) for activating side
   panel item and this causes some methods are called multiple times.
3. Renamed ambiguous`Sidebar::UpdateSidebar()` to `Sidebar::UpdateSidebarItemsState()`
   as it's called only when item's status should be updated.
4. Removed `SidebarController::UpdateSidebarVisibility()`. Instead of asking sidebar's
   visibility update from controller, `SidebarContainerView` updates its child view's
   visibility via active index change notification.
simonhong added a commit to brave/brave-core that referenced this issue Jul 7, 2023
fix brave/brave-browser#21992
fix brave/brave-browser#25382

Added slide animation for opening/closing sidebar/side panel.
To use slide animation, this PR has some more refactoring/cleanup.

1. Ask to show/hide sidebar/panel when only needed.
2. Use SidePanelUI(SidePanelCoordinator) for starting side panel activating.
   So far, we use both(SidePanelUI/SidebarController) for activating side
   panel item and this causes some methods are called multiple times.
3. Renamed ambiguous`Sidebar::UpdateSidebar()` to `Sidebar::UpdateSidebarItemsState()`
   as it's called only when item's status should be updated.
4. Removed `SidebarController::UpdateSidebarVisibility()`. Instead of asking sidebar's
   visibility update from controller, `SidebarContainerView` updates its child view's
   visibility via active index change notification.
simonhong added a commit to brave/brave-core that referenced this issue Jul 12, 2023
fix brave/brave-browser#21992
fix brave/brave-browser#25382

Added slide animation for opening/closing sidebar/side panel.
To use slide animation, this PR has some more refactoring/cleanup.

1. Ask to show/hide sidebar/panel when only needed.
2. Use SidePanelUI(SidePanelCoordinator) for starting side panel activating.
   So far, we use both(SidePanelUI/SidebarController) for activating side
   panel item and this causes some methods are called multiple times.
3. Renamed ambiguous`Sidebar::UpdateSidebar()` to `Sidebar::UpdateSidebarItemsState()`
   as it's called only when item's status should be updated.
4. Removed `SidebarController::UpdateSidebarVisibility()`. Instead of asking sidebar's
   visibility update from controller, `SidebarContainerView` updates its child view's
   visibility via active index change notification.
@brave-builds brave-builds added this to the 1.58.x - Nightly milestone Jul 12, 2023
@stephendonner stephendonner added the feature/user-interface All UI related label Aug 7, 2023
@stephendonner
Copy link

Verified PASSED using

Brave | 1.58.78 Chromium: 116.0.5845.51 (Official Build) nightly (64-bit)
-- | --
Revision | ca260d83d1ffda3cb35bff4bdc3251f30f9c8ccd
OS | Windows 10 Version 22H2 (Build 19045.3271)

Confirmed sidebar open/close operations are smoother/less janky. (I set framerate in my screen capture to 30 fps; it'll appear slightly smoother, even, in person.)

show-sidebar-smooth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/sidebar Relating to Brave's Sidebar feature feature/user-interface All UI related OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Win64 QA/Yes release-notes/include
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants