musescore / MuseScore

MuseScore is an open source and free music notation software. For support, contribution, bug reports, visit MuseScore.org. Fork and make pull requests!
https://musescore.org
Other
12.2k stars 2.64k forks source link

[MU4 Issue] Improving workflow of Panels (Palettes, Properties, Instruments and Selection Filter) #11218

Open KaiVinter opened 2 years ago

KaiVinter commented 2 years ago

Description: On MS3 you could view all the panels at once, even when stacked, so everything was pretty easily accessible. However in MS4, since the 4 main panels are stacked 'on tabs' on the same place, I think it would be a major improvement for the workflow to make them appear in the front whenever they are triggered, specially for people who are more used to use keyboard shortcuts.

I mean: If a panel is open but not visible (only tabbed) it should become visible whenever it's triggered, instead of having to trigger it twice for closing and then reopening. If a panel is visible, it should be closed when triggered.

Tantacrul commented 2 years ago

Can you clarify what you mean by 'triggered'?

Incidentally, the panel layout in MS4 is more flexible than three. We just started with a different default layout. MS3 also had tabs but not by default.

If you drag one of the tabs somewhere else, you'll notice you can stack them vertically, horizontally or move them to the other side of the screen.

Incidentally, we'll be releasing with extensive documentation, so users won't need to figure this out without a helping hand.

KaiVinter commented 2 years ago

With 'triggered' I mean called, [Ex. If I press F9 I 'trigger'/toggle the Palettes panel], probably a translation error from my side.

Sure you can stack them as in MS3, but I want to use tabs. And whenever I toggle/call a panel that is not shown in the front with a shortcut, it closes. And I think it makes sense that if I press a keyboard shortcut for a panel, it opens/shows in the front instead of closing.

Tantacrul commented 2 years ago

Oh, I see. That makes sense.

Tantacrul commented 2 years ago

OK, so I agree we should match the MS3 functionality:

For the following panels: Selection filter, Properties, Instruments & Palettes :

KaiVinter commented 2 years ago

Exactly.

In sketch code it would be something like:

if > pannels stacked in tabs {
    when > toggle panels {
        if > closed > then > open & show in the front.
        if > opened in another tab > then > show in the front.
        if > shown in the front > then > close.
    }
}

Thank you!

MarcSabatella commented 2 years ago

See also https://github.com/musescore/MuseScore/issues/12873 - the palette search command should also either open or "top" the palettes panel. Probably doesn't make sense for this command to ever close the panel though.

I believe it would also make sense that when opening or topping the panel, they also take keyboard focus. At least, I think that makes sense. In MU3, this was true for the Palettes but not for the Inspector. @shoogle ? Right now, I'm concerned that it's a very confusing story for a blind user trying to find these panels, as they don't necessarily know if they are currently open or closed, and pressing the shortcut doesn't seem to do anything in terms of something that would affect keyboard focus or provide screen reader feedback.

zmelson commented 2 years ago

Hello, I am new to this project and would like to contribute. Based on my understanding of the architecture, would this issue require updating the UI actions for the relevant shortcuts depending on the UI context? If so, under which modules are these panels located?

shoogle commented 2 years ago

@MarcSabatella, correct. If pressing the shortcut results in the panel being opened (or made the selected tab) then the panel should receive keyboard focus.

@zmelson, welcome! You probably won't need to modify the UI action, shortcut, or context. Instead, you need to look for the function that gets called when the corresponding action occurs. Using the Master Palette as an example, this function gets registered to the action code, so you can find the function by knowing the action code.

It's a bit more complicated for the Palettes Panel, but searching for the action code "toggle-palettes" took me to ApplicationUiActions::toggleDockActions(), and following the trail of where that function is used eventually leads to DockWindowActionsController::toggleOpened(), which is probably the one you need to modify (either that or the toggleDock() function mentioned inside).

zmelson commented 2 years ago

Thank you for the clarification and guidance @shoogle, it is very appreciated!

zmelson commented 2 years ago

Okay, I've been testing some things locally and I've traced the function calls down to DockPageView::setDockOpen(). I've tried manipulating some of the properties of the DockBase and DockPanelView objects and have been able to change whether the panels open or close. However, I am having trouble finding a property that shows whether a panel is currently selected or not. I think if I could get and update that property, I would be able to make the toggle act as desired. Would anyone happen to know if such a property exists in either the DockBase or DockPanelView classes?

zmelson commented 2 years ago

Never mind on the previous question, I ended up going one step deeper and found the property isCurrentTab() and the function setAsCurrentTab() under the DockWidget class. I ended up writing a function in the DockPanelView class and editing DockPageView::setDockOpen() to call this new function when relevant. I have it basically working but still need to clean it up on my end before submitting.

EDIT: I still need to distinguish between a shortcut toggle and toggling from the View drop-down menu. Currently both are selecting the tab when it is open but not selected, rather than just the shortcut.

s1m0n-github commented 2 years ago

Only slightly related, but #11242 got no recognition, but it's still open. Issue is that the panels won't remember their position when activating/deactivating dynamically, which is also pretty bad for the workflow :sweat_smile:

Thought I'd mention it under this topic

zmelson commented 2 years ago

Just to confirm, should the behavior of the shortcut commands and drop-down menu be different? That is, should the drop-down always close the panel if it is open, regardless of whether the panel is selected or not?

And if so, would that require the shortcut to trigger a different command than the drop-down (or at least have the relevant functions track whether a shortcut triggered them or not)?

Sorry for all the questions, I just want to make sure I am addressing the issue as stated.

jeetee commented 1 year ago

Now that I'm using 4 a lot more, this has been biting me as well (shortcut not brining the panel to the front of the tab stack).

Tantacrul commented 1 year ago

Yeah, it's annoying for sure. We've got it in 'Post Release' for now... which is pretty much our shortlist. Pretty damn long shortlist though 😄 (we have a lot of work in front of us!)

hanoixan commented 1 year ago

I've picked up this issue and submitted a PR. My fix handles horizontal tab panels as well as vertical tab panels.

hanoixan commented 1 year ago

I have a PR in review, but I'm having second thoughts about whether this meets all use cases and situations, so let me lay them out to get some consensus. @shoogle @MarcSabatella

Toggling a pane:

If the above is correct, I have some further questions:

  1. If you toggle the tab to be visible, drag it into a custom dock, and then toggle to remove it, and THEN toggle it again, it will appear in the original panel, not where you moved it to. I feel like this will be confusing and lead to situations where people hit the toggle hotkey because they don't see it initially, only to have it close and then reappear in the default place. It also confuses the story for blind users and keyboard focus.
  2. Is it possible that the better UX is to interpret a panel hotkey as an open/tab select only (with keyboard focus), and closing must be done with the mouse?
s1m0n-github commented 1 year ago

To Question 1: It totally will lead to confusing situations, especially when your workflow embraces the toggle feature to see more of your score (happened to me :P) I don't know how it is decided where a widget is drawn in the code, but maybe there's a possibility to log the position and recall it on toggle, or just make the panel invisible? Depends on how the panels and toggles are implemented right now

jeetee commented 1 year ago

@hanoixan All situations are correct except for the opening of a closed tab. When (re-)opening a closed tab it must reappear in the position and state where it was when closed.

  1. If it was floating, then it should reopen that way. Exception: if it was floating on a secondary screen that is now not available, it should be opened on an available screen instead, but still floating and preferable at the same position (if within the new screen boundaries).

  2. If it was docked, it should open at the original dock position. Even if it was tabbed with other panels that are now no longer at that position, this panel should reopen at its dock position where it was closed.

Re: closing with mouse only: imho the answer is no. Keyboard only closing of a panel should remain an option. Having the keyboard shortcut action act as the toggle makes for: a) less keyboard shortcuts b) quick toggle when checking a single thing (Properties Panel) c) (minor) consistency with MS3

bkunda commented 1 year ago

For the following panels: Selection filter, Properties, Instruments & Palettes :

  • If the panel is tabbed but hidden, then the shortcuts should display it (make it the selected tab) and not close it
  • If the panel is already selected and the user triggers the shortcut, then the panel should be closed

@hanoixan Thanks for working on this! I've had a look at #15675 and, as far as @Tantacrul's conditions (quoted above) are concerned, this behaviour is now working in your PR 👍🏻.

I agree with others in this conversation that the thing remaining to be addressed is that the position of a re-docked (or undocked) panel should persist, even when that panel is closed and reopened. This is really important.

Further, if a panel is re-docked in a custom position, and the user uses its shortcut to bring it into focus, it should not reposition the panel; I.e. its custom position (whether re-docked or floating) should persist. So:

This video addresses these two points:

https://user-images.githubusercontent.com/86290556/210363429-94812e5a-9282-4b90-9fdc-dda888cd3191.mov

I also found a weird bug in your PR, concerning specifically the palettes and instruments panels: at present, when either of these panels is repositioned, bringing the other into focus with a keyboard shortcut actually re-docks it to wherever the first panel was repositioned (seeing this video should hopefully make this clearer):

https://user-images.githubusercontent.com/86290556/210361381-34a040ae-ad9e-4abd-a226-af27fd7ca988.mov

Finally, the panel options present in the View menu should operate only to toggle their visibility (as per current behaviour in the stable v.4.0 build), not to bring them into focus as it does in your PR. I.e., these menu options should only close/open each panel.

Hope this is clear. Thanks again for your work on this! 🙏🏻

hanoixan commented 1 year ago

@bkunda and all, your responses are super clear, thank you, and I think this simplifies the design.

The original code does some extra work in findPanelForTab() that tries to be smart about the parent panel, but that should only be utilized if the tab doesn't have an existing parent (where it was last docked). I'll post when I update the PR.

The only thing that doesn't quite sit right is how there is no visual feedback when a hotkey focuses an already open panel, but I think that can be a future issue if it ends up being a problem.

bkunda commented 1 year ago

@bkunda and all, your responses are super clear, thank you, and I think this simplifies the design.

Thanks @hanoixan. We try our best to keep things simple 😊

The only thing that doesn't quite sit right is how there is no visual feedback when a hotkey focuses an already open panel, but I think that can be a future issue if it ends up being a problem.

I completely agree with you – we can address this if it ends up being problematic 👍🏻

Looking forward to your update!

hanoixan commented 1 year ago

@bkunda If I'm not mistaken, there is no existing functionality that tracks the last DockPanelView that a DockPanelView tab was a part of when it was last closed, nor does it track floating state of a closed panel.

So, the next step seems to be to implement this. Just so I'm clear, though, this tracking of the last tab only persists while the application is running. Once restarted, if the tab is toggled open, it will start in the default pane for that tab, correct?

bkunda commented 1 year ago

Just so I'm clear, though, this tracking of the last tab only persists while the application is running. Once restarted, if the tab is toggled open, it will start in the default pane for that tab, correct?

Let me just clarify that I've correctly understood the context of what you've written:

  1. A panel is re-docked to a new position, or is undocked and floating
  2. That panel is then closed
  3. User quits and re-opens the app
  4. User opens the same panel

In an ideal world, the last position of this panel would also be preserved, even after restarting the application. This would cover cases such as, for example, where the user temporarily closes a panel to increase their score view just before the app crashes; when they reopen the app, they would be able to return to their previous workspace layout without any interruption to their flow.

But this is admittedly a bit of an edge case, and I really don't think there's a massive problem with closed panels being returned to their default pane after the app is restarted if this simplifies the implementation of this functionality. The priority should be preserving the last remembered positions of panels when the app is running, so the user can effectively show/hide their panels while they work.

hanoixan commented 1 year ago

@bkunda I've pushed up some new changes, and went in a different direction than I thought it would. It turns out that panels do save their location for reopening, and we just had to get out of their way. It even works between sessions.

Skelbti commented 1 year ago

Something similar happens when you move the palette to the right of the screen. When you hide it and display it with the F9 key, it reappears on the left side of the screen. MuseScore4_xuYDct0BpI

dwlakes commented 7 months ago

I think I'd like to try this one. Going through the comments though, it seems like the end goal(s) have morphed over time. But essentially, it seems like we're wanting the Panels to "remember" where they were put by the user and and come "to the front" when their shortcut is selected, given that they are already not visible?

shoogle commented 7 months ago

@dwlakes, yes, that sounds right to me. When a panel is opened or comes to the front, it should also receive keyboard focus.