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

Library controller navigation fixes & enhancements #2333

Closed
wants to merge 5 commits into from

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Oct 25, 2019

I attempt to clean up my outdated previous PR to clear search via controllers with [Library],GoToItem, as well as some small improvements for library controller navigation.

reworked and rebased since noone reviewed it anyways so far
I hope the commits are easy to review because I think this solves some issues with basic Library navigation that would ease the library/controller UX in 2.3

https://bugs.launchpad.net/mixxx/+bug/1840541

don't allow to focus Library feature buttons
... in Recording, AutoDJ, Analysis, Hidden or Missing neither with Tab key on keyboard nor [Library]MoveFocus... to always see the focus indicating border on the sidebar or tracks table.
There's no point in having those focused as there's no way to trigger them, neither from keyboard or controllers. For AutoDJ and Recording there are keyboard shortcuts.

fix hasFocus()
... in DlgRecording, DlgAutoDJ, DlgAnalysis, DlgHidden and DlgMissing to prevent 'refocus Library' calls there before issuing a move/scroll command.

Prevent focus from getting stuck at Clear button
In master, the focus can be moved to the Clear button with [Library],MoveFocus[Backward, Forward] (when there is an active search). Then, MoveFocusForward would do nothing because behind the scenes library control tests if the sidebar or tracks table have focus; if false it would try to re-focus the tracks table: focus the sidebar > emulate Tab > then send the MoveFocus Tab = we end up at the Clear button again. With MoveFocus, -1 it would jump between sidebar and Clear button.
Now, the focus controls always send a Tab key no matter which widget has focus.

Clear Search from controller
Clear the seach with [Library],GoToItem when either the searchbox or the Clear button has focus.

ToDo

  • clean up, remove debug output
  • remove focusAnalysis()
  • resolve conflicts
  • add some skin feedback: dim border for WTextBrowser, ...

@ronso0 ronso0 force-pushed the lib-control-simple branch from 7f0bb35 to 4f9b050 Compare October 25, 2019 17:48
@ronso0 ronso0 force-pushed the lib-control-simple branch from 6f45f79 to e9fd03c Compare December 26, 2019 16:47
@ronso0 ronso0 force-pushed the lib-control-simple branch from 0b80598 to a1e996b Compare March 22, 2020 23:45
@ronso0 ronso0 changed the title [WIP] allow [Library],GoToItem to clear search and other small improvements [WIP] Library navigation fixes Mar 24, 2020
@ronso0 ronso0 self-assigned this Mar 24, 2020
@ronso0 ronso0 force-pushed the lib-control-simple branch from a1e996b to 185984e Compare March 25, 2020 00:12
if (m_pLibraryWidget->getActiveView()->hasFocus()) {
return true;
}
return false;
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe this doesn't need to be a separate function, until now we need it only twice here.

} else {
// Otherwise toggle the sidebar item expanded state
slotToggleSelectedSidebarItem(v);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

unchanged

// Clear the search if the searchbox has focus
if (m_pSearchbox->hasFocus()) {
return emit(clearSearch());
}
Copy link
Member Author

Choose a reason for hiding this comment

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

unchanged

@ronso0 ronso0 changed the title [WIP] Library navigation fixes Library controller navigation fixes Mar 25, 2020
@ronso0 ronso0 changed the title Library controller navigation fixes Library controller navigation fixes & enhancements Mar 25, 2020
@ronso0 ronso0 requested review from daschuer and uklotzde March 25, 2020 00:32
@ronso0 ronso0 added this to the 2.3.0 milestone Mar 25, 2020
@ronso0
Copy link
Member Author

ronso0 commented Mar 25, 2020

After GoToItem > Clear Search we could also automatically jump to the tracks table.
I guess someone clearing the search via controller wants to see and scroll all available tracks with it, rather than refining the search which she/he needs the keyboard for anyway.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 25, 2020

I guess someone clearing the search via controller wants to see and scroll all available tracks with it, rather than refining the search which she/he needs the keyboard for anyway.

I don't think that's safe to guess. Clearing the search from a controller then reaching to the keyboard to type seems like a reasonable thing to do IMO.

@ronso0
Copy link
Member Author

ronso0 commented Mar 25, 2020

don't allow to focus Library feature buttons
... in Recording, AutoDJ, Analysis, Hidden or Missing neither with Tab key on keyboard nor [Library]MoveFocus... to always see the focus indicating border on the sidebar or tracks table.
There's no point in having those focused as there's no way to trigger them, neither from keyboard or controllers. For AutoDJ and Recording there are keyboard shortcuts.

Any objections?
Tab / MoveFocus now always moves focus between tree view and tracks tables (and Clear search button). No need to Tab through a row of buttons anymore, like in AutoDJ.
BUT I don't know if someone actually makes use of Tab, Enter and Space to navigate library features like web pages, maybe a visually impaired user? if at all, this wouldn't work now anymore. we can revert that commit later on. Edit and add focus highights to every skin

@ronso0
Copy link
Member Author

ronso0 commented Mar 25, 2020

Closed and split up for easy review
#2597
#2598
#2599

@ronso0 ronso0 closed this Mar 25, 2020
@ronso0 ronso0 deleted the lib-control-simple branch March 25, 2020 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants