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

Navigation Control Border Re-Implementation #8806

Merged
merged 3 commits into from
Aug 9, 2021

Conversation

ArjunBoi
Copy link
Contributor

@ArjunBoi ArjunBoi commented Aug 6, 2021

Resolves: #8442

All elements supporting Navigation Control now draw the border rectangle on the outside.
There were a total of 5 different elements i.e.

  • Dropdown: (it works for the main Dropdown item, but the items inside it are very close together. I have implemented this in such a way that the borders for the popup elements are drawn on the inside).
  • DockPanelTab : The borders are being drawn on the inside, again because of lack of space.
  • InstrumentsTreeItemDelegate : Borders on the inside, same reason.
  • PaletteTree : This file uses ListItemBlank as a delegate to draw its children. I've done it in such a way such that only this instance of ListItemBlank 's borders get drawn on the inside.
  • Expandable Blank : Same thing

The borders for these are drawn on the inside, in a way that it does not interfere with the High-Contrast borders; i.e. the HC 1px border + the 2px navCtrl border will combine to make a single 3px border on the inside (the innermost being the HC border, followed by the navCtrl one).

For a demonstration, Navigate to DevTools -> KeyNav. (Please make sure that you are in Light mode. The colors for the elements in that page are hard-coded and don't work well with other themes)

This PR does not modify the existing Navigation Control logic for elements, just the way the borders are drawn around them.

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • I created the test (mtest, vtest, script test) to verify the changes I made

@ArjunBoi ArjunBoi force-pushed the NavigationControlBorders branch from 64aa64e to 28f74a4 Compare August 6, 2021 12:55
@ArjunBoi ArjunBoi force-pushed the NavigationControlBorders branch from 28f74a4 to 13ae79e Compare August 6, 2021 13:01
@ArjunBoi
Copy link
Contributor Author

ArjunBoi commented Aug 6, 2021

There are a lot of bugs within the Navigation Control system that made testing very troublesome.
There were elements that were mentioned only in a few places in the code, which were out of the reach of NavigationControl and thus weren't test-able immediately.

In other places, the application would often crash trying out the different options. I also encountered behavior in which i would press space to select an option in a group of FlatRadioButtons, and the application wouldn't update the changes made because of having that button selected. The changes were only made once i clicked that button with my mouse. In the same element, pressing "Enter" instead of "Spacebar" led to a crash. (I found this behavior in the Text settings in the Inspector/Properties tab)

@ArjunBoi ArjunBoi force-pushed the NavigationControlBorders branch from 609e725 to 82bb2b4 Compare August 8, 2021 00:05
@ArjunBoi ArjunBoi force-pushed the NavigationControlBorders branch from 82bb2b4 to ceed967 Compare August 8, 2021 17:21
@ArjunBoi ArjunBoi requested a review from RomanPudashkin August 8, 2021 17:22
@RomanPudashkin RomanPudashkin merged commit 7f061a6 into musescore:master Aug 9, 2021
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.

[MU4 Issue] Tabbing outline stroke needs to sit on the outside of each element (currently it is inside)
2 participants