Fix toggle menus to adhere to HIG, #3116 #3980
Conversation
This commit will change these toggle menus to use "Hide" in their title instead of "Show" when selecting the menu item hides the panel: - Show Chapters Panel - Show Playlist Panel - Show Quick Settings Panel This commit adds text that will require localization. This fix only corrects one part of the reported issue. The behavior of buttons still needs to be updated to adhere to the HIG.
There was a problem hiding this comment.
The inconsistency annoys me. Currently there are Show Playlist Panel and Show Chapter Panel in the playback menu, but in it is Show Quick Settings Panel in Video, Audio, and Subtitle menu.
We should rename them into Show Video/Audio/Subtitle Panel and Hide Quick Settings Panel correspondingly.
|
The inconsistency in menu item names is due to the inconsistency in the menu structure. Using Video, Audio and Subtitles in the item names seems like duplicate text. Menu items according to the HIG are supposed to use "a condensed style". Of course you can argue that we are already violating that with items such as "Audio filters...". I can go ahead and make this change and we can see how it looks. At the moment I am distracted by the fact that this fix is not working. MainWindowController has changed. The sideBarStatus used to indicate which panel was set, .settings or .playlist. That is no longer working. It is set to .settings when the playlist/chapter panel is being displayed. So the menus are malfunctioning. Was hoping to post an additional commit that as requested adds the english translations. But I need to investigate why the sideBarStatus seems to be incorrect. A quick fix did not work. Need to dig into what happened in the MainWindowController code. Tomorrow busy. Might be able to post an update Saturday. |
Sorry I didn't make it clear. I meant we should rename them into
|
|
I picked up the commit and confirmed that fixes the problem. |
This commit will change these toggle menus to use "Hide" in their title instead of "Show" when selecting the menu item hides the panel: - Show Chapters Panel - Show Playlist Panel - Show Quick Settings Panel In addition the Show Quick Settings Panel menu item in the Video, Audio and Subtitles menu have been renamed to reflect the type of settings: - Show Video Panel - Show Audio Panel - Show Subtitles Panel This commit adds text that will require localization. This fix only corrects one part of the reported issue. The behavior of buttons still needs to be updated to adhere to the HIG.
|
I've renamed the menu items. This does create a slight inconsistency with the buttons used in the OSC: Should |
|
What’s the inconsistency? |
|
I remembered this morning that I forgot to update MainMenu.xib. Even though the text in the xib is not used it should be updated to not confuse developers. So that needs to be changed before this PR is ready to be merged. On the question about the button inconsistency... Currently the menu items are:
And the OSC buttons are:
So it is clear that the By renaming the menu items to make them more consistent:
We have eliminated the term "Quick Settings" and broken the correspondence with the OSC button. If we want to proceed with the change to the menu items then I think the buttons should be renamed as well:
Should I go ahead and change the button name as well? |
No, "Quick Settings" means the video panel + audio panel + subtitle panel. |
|
How does the user know what "Quick Settings" means now that we removed that text from the menus? The panel itself has no such label. That was my worry. Should I also update the menu item text in MainMenu.xib? It gets overwritten, but if I leave it set to "Show Quick Settings Panel" it could confuse a developer. |
I think this is a valid concern, but since we have used "Quick Settings" for years in IINA, removing that will also surprise some of the users. I think we can discuss this in another issue, let's get this merged first!
Yes please do that. That should change the corresponding text in |
This commit will change these toggle menus to use "Hide" in their title instead of "Show" when selecting the menu item hides the panel: - Show Chapters Panel - Show Playlist Panel - Show Quick Settings Panel In addition the Show Quick Settings Panel menu item in the Video, Audio and Subtitles menu have been renamed to reflect the type of settings: - Show Video Panel - Show Audio Panel - Show Subtitles Panel This commit adds text that will require localization. This fix only corrects one part of the reported issue. The behavior of buttons still needs to be updated to adhere to the HIG.
|
I've pushed a commit that updated the menu item text in MainMenu.xib and en.lproj/MainMenu.strings. Of course that text is overwritten by Surprised that the toggle concept is not directly supported by the framewor/XIB.. If there is a way I could not find it. I only found packages from people that created components that were layered on top of the framework to encapsulate the toggle concept. |
This commit will change these toggle menus to use "Hide" in their title instead of "Show" when selecting the menu item hides the panel:
This commit adds text that will require localization.
This fix only corrects one part of the reported issue. The behavior of buttons still needs to be updated to adhere to the HIG.
Description:
This PR replaces the original PR, removing localization files.