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

lp1719474 Follow-up: Fix navigation usability issues in sidebar tree #1620

Merged
merged 2 commits into from
Apr 21, 2018
Merged

lp1719474 Follow-up: Fix navigation usability issues in sidebar tree #1620

merged 2 commits into from
Apr 21, 2018

Conversation

uklotzde
Copy link
Contributor

@uklotzde uklotzde commented Apr 17, 2018

From the discussion in #1619

A timeout of 250300 ms still feels responsive and scrolling slowly through the list of playlists and crates is possible. This new timeout is now used for every LibraryFeature as a default, unless customized in a derived class!

@uklotzde uklotzde changed the title lp1719474 Follow-up: Use a default timeout of 250 ms instead of 100 ms [WiP] lp1719474 Follow-up: Use a default timeout of 250 ms instead of 100 ms Apr 17, 2018
@uklotzde
Copy link
Contributor Author

I've found a better solution by correctly distinguishing clicks and key presses. This avoids the timeout when clicking and we can choose a timeout that works best just for scrolling.

I also forgot to mention that this usability issue also effects keyboard navigation within the sidebar tree, i.e. you can test the behavior without a controller just with your keyboard!

Stay tuned...

@uklotzde uklotzde changed the title [WiP] lp1719474 Follow-up: Use a default timeout of 250 ms instead of 100 ms lp1719474 Follow-up: Fix navigation usability issues in sidebar tree Apr 17, 2018
@uklotzde
Copy link
Contributor Author

  • Cleaner and simpler code
  • A single constant for consistent behavior within all feature sections
  • No more collateral damage = additional latency when using the mouse

I've increased the timeout to 300 ms which works very well for me in most situations.

@uklotzde uklotzde added this to the 2.1.1 milestone Apr 18, 2018
@uklotzde
Copy link
Contributor Author

Did a forced update to avoid changes in WLibrarySidebar that were unnecessary.

@daschuer
Copy link
Member

Works good and LGTM.

@daschuer daschuer merged commit 8ad4048 into mixxxdj:2.1 Apr 21, 2018
@uklotzde uklotzde deleted the lp1719474_followup branch April 22, 2018 22:10
@@ -210,6 +210,8 @@ void Library::bindSidebarWidget(WLibrarySidebar* pSidebarWidget) {
connect(m_pSidebarModel, SIGNAL(selectIndex(const QModelIndex&)),
pSidebarWidget, SLOT(selectIndex(const QModelIndex&)));
connect(pSidebarWidget, SIGNAL(pressed(const QModelIndex&)),
m_pSidebarModel, SLOT(pressed(const QModelIndex&)));
connect(pSidebarWidget, SIGNAL(clicked(const QModelIndex&)),
Copy link
Member

Choose a reason for hiding this comment

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

This change causes that the activate() is called twice.

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