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

Add option and keybinding to hide menubar #2972

Closed
wants to merge 1 commit into from

Conversation

xiota
Copy link
Contributor

@xiota xiota commented Oct 29, 2021

This PR adds an option to hide the menubar at startup and a keybinding to toggle menubar visibility. The menubar is hidden only if a keybinding to reshow it has been set. A message is shown in the status window to remind the user of the keybinding setting. When the option to hide at startup is disabled, the menubar state is saved and restored.

Notes:

@elextr
Copy link
Member

elextr commented Oct 29, 2021

As @b4n said on #633 it should not be bound by default.

Keybindings can be typed by accident, (like you accidently holding ctrl when scrolling, someone accidently holding ctrl when typing m) and people accidentally hiding their menubar and not knowing how to get it back is a "bad thing" ™️. Whilst I still think it should have a confirmation dialog, at least if its a user defined key they can be expected to remember it.

Also if its not stolen by the desktop <Alt>h s will show a list of keybindings for those with faulty ... erm, you know, thingys ... oh yeah memories. But when the menubar is hidden thats also not discoverable.

@xiota
Copy link
Contributor Author

xiota commented Oct 30, 2021

@elextr The problem was that if the user sets the menubar to hide on startup without having a keybinding to reshow the menubar, it becomes very difficult to reshow the menubar. (The config file needs to be located and edited, but if the user tries to use Geany to edit the config, it gets overwritten with the old setting.)

The problem is avoided now by not hiding the menubar if the keybinding to reshow it hasn't been set.

Confirmation dialogs would be very annoying to users who would actually want to use this feature. I suppose I can add more warning_on_* settings. I'll add some status window messages with the keybinding to reshow the menu.

@elextr
Copy link
Member

elextr commented Oct 30, 2021

Why not save the menubar status and restore it on restart instead of having a separate setting. Then the user has to have set the keybinding to turn it off.

@xiota
Copy link
Contributor Author

xiota commented Oct 30, 2021

Why not save the menubar status and restore it on restart instead of having a separate setting. Then the user has to have set the keybinding to turn it off.

(Based on a small sample) Users that want the menubar hidden want it hidden at startup, regardless of the state it was in when the program was last used. (If it saved state, I'd expect complaints about the menu not staying hidden on startup.)

@elextr
Copy link
Member

elextr commented Oct 30, 2021

Users that want the menubar hidden want it hidden at startup

Well that would make it the only UI element that behaved that way.

All other UI elements save their state and restart Geany in the same state so the user can continue their work where they left off. Having a UI element change state between Geany shutdown and restart would be unique.

It is better for similar functions (in this case setting UI layout, eg show/hide message window or sidebar or "additional widgets" and now menubar) to behave the same way, the users can generalise behaviour rather than having to remember that feature X behaves differently. For goodness sake, they might have to read the fine manual if that was the case. 😜

Fitting user requests into the application consistently is an important consideration for developers, not necessarily taking the first thing someone thought of (and that includes the first thing developers think of too). 😁 Thats why there is discussion around features, the second person that sees the idea has not had to use the brainpower to invent the idea from scratch, so will often see improvements or alternates that the originator missed. Discussions should preferably happen before implemention so it only has to be done once.

@xiota
Copy link
Contributor Author

xiota commented Oct 30, 2021

Well that would make it the only UI element that behaved that way.

The menubar could save state by default (to make devs happy). But I'm reasonably sure someone (who isn't me) would open an issue (shortly after it becomes available in package repositories) to hide the menu on startup. Then another PR would be needed in another five years.

There are other settings/behaviors that reset on restart. And no one has to enable options they don't want.

It is better for similar functions (in this case setting UI layout, eg show/hide message window or sidebar or "additional widgets" and now menubar) to behave the same way...

The toolbar, message window, status bar, etc can be shown/hidden and left that way. But occasional access to the menu is needed to access Geany functions. For a user who wants the menu hidden, it would be an annoyance to have the menubar visible at startup because it wasn't rehidden after previous use. (Autohide... but will leave that for a different PR.)

... the users can generalise behaviour rather than having to remember that feature X behaves differently.

In this case, it's a setting that the user would have explicitly set. If they can't remember that, they have greater problems.

@xiota
Copy link
Contributor Author

xiota commented Oct 30, 2021

@elextr The menubar state is now saved and restored. The option to hide on startup is kept because that is the behavior I expect most users who would hide the menu bar would want. I will update the initial description.

Discussions should preferably happen before implemention so it only has to be done once.

Too much discussion, and it may be several years before implementation. A concrete implementation can bring up issues that might not come up otherwise. (The issue had been open for many years without anyone mentioning the potential implementation issues that have come up after the PR was open.) There's no guarantee that an implementation with lots of discussion won't need to be revised.

Note: First version of this message said "not" instead of "now". Sorry for the terrible typo.

@xiota
Copy link
Contributor Author

xiota commented Nov 4, 2021

@elextr Would you prefer this be in a plugin?

@elextr
Copy link
Member

elextr commented Nov 4, 2021

I guess being in a plugin makes it harder to do lose the menubar accidentally, so plugins maybe a good idea. And as I have said elsewhere plugins have greater freedom to do things we have concerns about being in Geany.

@xiota
Copy link
Contributor Author

xiota commented Nov 4, 2021

@elextr Do you think it is "better" to have plugins that just do one thing (hide the menubar) or that do more (interface tweaks). (I know it's up to the plugin author, but I'm wondering about your perspective.)

@elextr
Copy link
Member

elextr commented Nov 4, 2021

Unfortunately I find there is a bit of effort involved in adding a new plugin to the GP collection, its fine if you are an autotools expert, but the last time I tried I broke it so bad several other people jumped in to help 😄 so the overhead may be too much for a really small "one liner" plugin. That has resulted in some things being tacked on to other plugins to avoid the overhead, thats fine so long as the grouping makes sense, such as your proposals for projectorganiser. But add-ons has become a bit of a catch all, which may not be ideal.

A "tweakui" plugin that can make more aggressive or experimental adjustments than Geany itself is comfortable with sounds like a good collection though. Its pretty common in other places so users will likely find it.

@xiota
Copy link
Contributor Author

xiota commented Nov 4, 2021

Unfortunately I find there is a bit of effort involved in adding a new plugin to the GP collection, its fine if you are an autotools expert, but the last time I tried I broke it so bad several other people jumped in to help smile so the overhead may be too much for a really small "one liner" plugin.

I just tried to clone a plugin with a different name and couldn't figure it out. How is it with adding a plugin that's included with geany?

That has resulted in some things being tacked on to other plugins to avoid the overhead, thats fine so long as the grouping makes sense, such as your proposals for projectorganiser. But add-ons has become a bit of a catch all, which may not be ideal.

I don't understand most of the options in the addons plugin... but looking at it again... It has two options that are relevant to another discussion:

  • Show a calltip when hovering over a color value
  • Open Color Chooser when double-clicking a color value

@elextr
Copy link
Member

elextr commented Nov 4, 2021

I just tried to clone a plugin with a different name and couldn't figure it out.

My recommendation is to first build the plugin as a separate plugin and make it build and work with normal autotools, its nice if it works on all platforms, but thats not essential so long as it doesn't build on platforms it won't work on. HACKING and the plugin docs are the reference for that. Plugin docs has a getting started.

You can probably even push that to your own github and see if you can get some of the interested parties to test it.

Once it works as a standalone plugin then integration with GP or Geany can be considered. You will more likely get help if your plugin is working already.

@xiota
Copy link
Contributor Author

xiota commented Nov 4, 2021

Well... I have xiota/geany-tweaks, which is basically just anything that I think doesn't fit in xiota/geany-preview. I've opened PRs for Geany when I think features fit better within Geany, aren't really possible to do in a plugin (#2899, #2949, #2951, #2952, #2953, #2954, #2955, #2979), or address open issues (#2919, #2956, #2972, #2973).

This one, and another one that I have in mind, can be done in a plugin though. Maybe adding to addons isn't too bad if the preferences are reorganized and documentation makes the features more visible.

@elextr
Copy link
Member

elextr commented Nov 4, 2021

Oh, ok, if you already have a plugin then its a case of trying to get it to work in a github fork of geany-plugins then you can make a PR for GP to add it.

IIUC you need to take out your configure.ac since there is one at the top level and you add your plugin to the list in it. Checks you need done are in the pluginname.m4 in the build directory and that also enables/disables the plugin if checks fail or its disabled on the command line. Its all explained in the geany plugins HACKING as well as the conventional makefile arrangement.

Maybe adding to addons isn't too bad

You need to talk to @eht16 about that, its his plugin.

But don't break it, its the only one I have enabled most of the time, the TODO feature is essential!!!! but I do wish my files have less of them 😁

@eht16
Copy link
Member

eht16 commented Nov 7, 2021

Maybe adding to addons isn't too bad

You need to talk to @eht16 about that, its his plugin.

I wouldn't say it is "my" plugin, I just wrote it initially :D.
while the Addons plugin could catch this feature as well (as it was said, it's already a catch all plugin and maybe already too big), I think the menubar feature fits rather in core Geany because it would align cleanly with the other show/hide options for interface elements like Message Window, Toolbar and Sidebar.

But don't break it, its the only one I have enabled most of the time,

Heh, the best reason to not break it: Lex is using it :D

@elextr
Copy link
Member

elextr commented Nov 7, 2021

I wouldn't say it is "my" plugin, I just wrote it initially :D.

By owner I mean maintainer.

Heh, the best reason to not break it: Lex is using it :D

Is there some other reason? ;-P

Back to the topic, the real problem with hiding the menu is when someone does it accidentally and doesn't know how to get it back. If it was a unbound toggle keybinding I think its fine, the user has to set something to enable it and hopefully will remember their keybinding.

But the "hide on startup" option does not work that way. Once its set the menu always starts hidden, and then without the menu its then very hard to discover how to bring it back. Thats why I suggested that this option needs to be in a plugin so its somewhat protected and starting Geany with -p will mean the plugin won't be loaded and won't hide the menu so the settings can be fixed.

@eht16
Copy link
Member

eht16 commented Nov 7, 2021

Back to the topic, the real problem with hiding the menu is when someone does it accidentally and doesn't know how to get it back. If it was a unbound toggle keybinding I think its fine, the user has to set something to enable it and hopefully will remember their keybinding.

But the "hide on startup" option does not work that way. Once its set the menu always starts hidden, and then without the menu its then very hard to discover how to bring it back. Thats why I suggested that this option needs to be in a plugin so its somewhat protected and starting Geany with -p will mean the plugin won't be loaded and won't hide the menu so the settings can be fixed.

Got it.
Though I don't think it makes much difference if the accidentaly hidden menu bar can be restored by deactivating plugins or by discovering the keybinding. Both ways require the user to take action and to read the documentation or ask for help.

I agree that the "hide on startup" option is not necessary. If implemented with a keybinding, a menu item in the View menu
and an option in the preferences dialog (on the Interface tab, not as Various pref), then it is more visible and via the preferences dialog easier to show the menu bar again. And it is in line with the other show/hide options for Toolbar, Sidebar and Message Window.

We could even add a FAQ item to explain how to restore the menu bar.

@xiota
Copy link
Contributor Author

xiota commented Nov 7, 2021

But the "hide on startup" option does not work that way.

@elextr The current implementation does not hide the menubar unless a keybinding is set. That applies to hide on startup. When hide on startup is set, but no keybinding is set, a message will be displayed in the status window to notify the user that a keybinding must be set before the menubar will be hidden.

the real problem with hiding the menu is when someone does it accidentally and doesn't know how to get it back. If it was a unbound toggle keybinding I think its fine, the user has to set something to enable it and hopefully will remember their keybinding.

By default, there is no keybinding set, so the user would not be able to hide the menubar accidentally. Even if a keybinding is set and the user forgets what it is, a message is sent to the status window when the menubar is hidden to remind the user how to restore the menubar.

I agree that the "hide on startup" option is not necessary.

@eht16 Hide on startup is a primary use case that this PR is intended to address. Restore on startup is basically just to make devs happy. I already have a couple plugin implementations (Geanypy, Lua script, and C plugin). I rewrote part of the Lua script to restore on startup while helping a user to get it working. The first request the user made after getting the script to work was to make it always hide on startup. (As far as I know, the script has only one user. So while it may not be representative of all users, the use-case does exist beyond just me.)

If a plugin would be needed anyway to hide on startup, it would be better for for the plugin to control all menubar hiding features. Dividing this task between Geany and a plugin introduces conflicts that complicate the plugin and can confuse users.

If implemented with a keybinding, a menu item in the View menu

Adding an option on the view menu would make it more likely for users to accidentally hide the menubar without being able to restore it. (The option will be inaccessible after the menubar is hidden.)

The current implementation prevents accidentally hiding the menubar by requiring a keybinding to be set. That would block the (hypothetical) menu option from working, which could lead to user confusion/complaints that the option does not work.

and an option in the preferences dialog (on the Interface tab, not as Various pref), then it is more visible and via the preferences dialog easier to show the menu bar again. And it is in line with the other show/hide options for Toolbar, Sidebar and Message Window.

This would make the feature more visible, and prone to accidental usage. Since at least occasional access to the menubar is essential, hiding it should be an advanced feature that only sufficiently motivated users will find and use.

We could even add a FAQ item to explain how to restore the menu bar.

I wouldn't mind writing it.

Menubar is hidden only when a keybinding has been set to reshow it.
Message is set to the status window when the menubar is hidden.

Menubar can be hidden on startup (subject to keybinding config).
Otherwise, previous shutdown state will be restored.
@xiota
Copy link
Contributor Author

xiota commented Nov 7, 2021

Force push to rebase and combine commits.

@eht16
Copy link
Member

eht16 commented Nov 7, 2021

I agree that the "hide on startup" option is not necessary.

@eht16 Hide on startup is a primary use case that this PR is intended to address. Restore on startup is basically just to make devs happy. I already have a couple plugin implementations (Geanypy, Lua script, and C plugin). I rewrote part of the Lua script to restore on startup while helping a user to get it working. The first request the user made after getting the script to work was to make it always hide on startup. (As far as I know, the script has only one user. So while it may not be representative of all users, the use-case does exist beyond just me.)

I understood the use case and of course, if the menubar is hidden once, it should be hidden again after restart. What I meant is that we just need to store the state (visible or hidden) and restore that state on startup. If we would implement it like the other interface component toggles (Sidebar, Toolbar, ...), it would be as easy as just a couple of copy&paste's and for the user it would be a consistent experience to show/hide various parts of the interface.

If implemented with a keybinding, a menu item in the View menu

Adding an option on the view menu would make it more likely for users to accidentally hide the menubar without being able to restore it. (The option will be inaccessible after the menubar is hidden.)
[...]
This would make the feature more visible, and prone to accidental usage. Since at least occasional access to the menubar is essential, hiding it should be an advanced feature that only sufficiently motivated users will find and use.

We can't completely prevent a user might hide the menubar accidentally. But if users know there is the possibility to show/hide the menubar, I assume it would be easier for them to discover how to show it again. And more users can use this at all as a menu item in the View menu and an option in the preferences dialog is easier to discover than a keybinding.

@elextr
Copy link
Member

elextr commented Nov 7, 2021

I understood the use case and of course, if the menubar is hidden once, it should be hidden again after restart. What I meant is that we just need to store the state (visible or hidden) and restore that state on startup. If we would implement it like the other interface component toggles (Sidebar, Toolbar, ...), it would be as easy as just a couple of copy&paste's and for the user it would be a consistent experience to show/hide various parts of the interface.

Agree with @eht16, see #2972 (comment)

And more users can use this at all as a menu item in the View menu and an option in the preferences dialog is easier to discover than a keybinding.

Agreed, I have been assuming mnemonics work even if the menu is hidden, @xiota could you check?

If they do then having a menu item means there is a fixed activation that can be documented. If the only way to make the menubar visible is a user settable keybinding then it can't be documented because nobody knows what the user set it to.

@elextr
Copy link
Member

elextr commented Nov 7, 2021

@xiota the test is to hide the menu bar and try <alt>h s and see if the keyboard shortcuts dialog shows.

@xiota
Copy link
Contributor Author

xiota commented Nov 8, 2021

@elextr

I have been assuming mnemonics work even if the menu is hidden

Menubar mnemonics do not work when the menubar is hidden.

If the only way to make the menubar visible is a user settable keybinding then it can't be documented because nobody knows what the user set it to.

You previously appeared to argue against having a pre-set keybinding because users might accidentally hide the menubar without knowing how to reshow it. That is the reason I would prefer it to be omitted from the menu itself.

@eht16

What I meant is that we just need to store the state (visible or hidden) and restore that state on startup.

This PR already implements saving and restoring the menubar state.

Menubar use is different from sidebar and msgwin use. Users can hide the sidebar/msgwin and forget about them. But the menu is needed to access features that don't have a keybinding set. Users may need to show the menubar to access features that don't have a keybinding set. If they forget to rehide the menubar before closing Geany, the menubar will be reshown on startup.

Since it's known that users will want the menubar to be able to (always) "hide on startup", it doesn't make sense not to include it (as an option – no one is forced to use it if they don't want to). If it is omitted, I am certain that there will be a complaint (from someone who isn't me) in a few years (or however long it takes for the feature to reach package repositories).

If we would implement it like the other interface component toggles (Sidebar, Toolbar, ...), it would be as easy as just a couple of copy&paste's and for the user it would be a consistent experience to show/hide various parts of the interface.

Since the menubar is different from the other interfaces, it should not be treated exactly the same. In particular, hiding it without a way to reshow it will cause problems for users.

We can't completely prevent a user might hide the menubar accidentally.

This PR takes measures to prevent accidentally hiding the menubar, one of which is to make the feature not too easy to find.

But if users know there is the possibility to show/hide the menubar, I assume it would be easier for them to discover how to show it again.

Depends on what you mean by "easier". The user might realize more quickly that it was hidden by a built-in feature, not some weird glitch. But hidden menubars are pretty common nowadays, so the difference may not be significant. Once the menubar is hidden, if there is no keybinding, the easiest way to reshow it is to edit the config file with a different editor (otherwise the changes would be overwritten when Geany saves state).

And more users can use this at all as a menu item in the View menu ...

When no keybinding is set. What do you expect to happen if the menu is used to hide the menubar?

and an option in the preferences dialog is easier to discover than a keybinding.

What option would you add to the preferences dialog if the "hide on startup" option is removed?

There is no need for a "restore state" option in preferences. There also isn't that much consistency among the other UI elements.

  • message window: in menu, but not preferences
  • status bar: in preferences, but not menu
  • sidebar: both
  • toolbar: both

The options in the preferences are in different sections/tabs – not easy to find. Since preferences is pretty cluttered and they're just duplicates of the menubar toggles, it might be good to remove them from preferences.

@xiota xiota closed this Nov 9, 2021
@xiota xiota deleted the pr-hide-menu branch November 9, 2021 03:08
@xiota xiota mentioned this pull request Jun 10, 2022
@ralf3u
Copy link

ralf3u commented Mar 13, 2024

@elextr Excuse me to disturb you because of an old subject, but you wrote some years ago at #2972 (comment):

I guess being in a plugin makes it harder to do lose the menubar accidentally, so plugins maybe a good idea.

So, my question is: Why Toggle menu bar is not allowed as plugin?

@elextr
Copy link
Member

elextr commented Mar 13, 2024

So, my question is: Why Toggle menu bar is not allowed as plugin?

Well the plugin proposed was geany/geany-plugins#1138 which was then closed because it didn't get any traction. Presumably none of the Geany-plugins devs had enough interest in those capabilities or enough time available at that point to examine it, and as no potential or interested users commented, and none of them tested it, there was no pressure to find time. And then the originator deleted it so now nobody can try it at all.

So I wouldn't say it was not "allowed", just that nobody other than the originator showed any interest, so they removed it.

@xiota
Copy link
Contributor Author

xiota commented Mar 14, 2024

none of the Geany-plugins devs had enough interest in those capabilities or enough time available at that point to examine it

Features that devs have no interest in have low chance of being added. This applies to most projects.

Would rather devs evaluate bug-fix PRs than new features few others want.

There are other ways to hide the menu bar:

  • Global / dbus menus and libraries. Plasma 5/6 have one included. XFCE has a widget that can be added to the panel. I've seen packages for other desktops, but have not tested them.

  • Via a script with the GeanyLua plugin.

so now nobody can try it at all

The plugin is still available in its own repo (link in comment to other PR). GitHub also keeps records of all PRs that sufficiently motivated users can use to build.

@elextr
Copy link
Member

elextr commented Mar 14, 2024

none of the Geany-plugins devs had enough interest in those capabilities or enough time available at that point to examine it

Features that devs have no interest in have low chance of being added. This applies to most projects.

Yes, thats true of volunteer projects like Geany and even commercially funded projects like vscode, it has 382 PRs, the oldest 2016, and 5k+ issues. Paid devs can be directed to work on something they are not personally interested in, but somewhere the decision has to be made what they are directed to, and for commercial projects its probably commercial interest :-)

The plugin is still available in its own repo (link in comment to other PR). GitHub also keeps records of all PRs that sufficiently motivated users can use to build.

Ahh, ok, I missed the link, I only saw the PR branch being deleted which meant the standard process would not work.

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.

Hide Menu Bar
4 participants