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

New Widget: MenuBar #169

Merged
merged 13 commits into from Aug 12, 2020
Merged

New Widget: MenuBar #169

merged 13 commits into from Aug 12, 2020

Conversation

shundhammer
Copy link
Contributor

@shundhammer shundhammer commented Jul 9, 2020

Trello

https://trello.com/c/4GtDpmMo/

Bugzilla

https://bugzilla.opensuse.org/show_bug.cgi?id=1175115

Overview

This adds a MenuBar widget to the YaST UI.

Features:

  • A menu bar that spans the width of the parent widget
  • Pulldown menus
  • Menu items that can trigger an action
  • Separators
  • Submenus
  • Enabling / disabling individual items or complete menus / submenus
  • Optional icons for items and submenus
  • Automatic keyboard shortcut resolving on each menu level

Caveat

We normally use wizards everywhere which is a completely different approach to user interfaces. In certain cases, though, a traditional menu bar with pulldown menus is still useful.

We have it in the PackageSelector (which is a high-level standalone widget that implements it in pure Qt or pure NCurses), but it would also be desirable to have it in the partitioner which has a lot of actions that are right now all organized in buttons or MenuButtons.

Screenshot

MenuBar1-rb

Details

Creating Menus

Add nested items, very much like for the tree widget.

Separators

Use an item with an empty label or with a label that starts with --- (three "minus" signs).

Enabling / Disabling Items

Create a hash with the IDs of the items that are to be enabled or disabled and a boolean for the new enabled/disabled status and call UI.ChangeWidget(:menu_bar_id, :EnabledItems, my_hash) with it:

      UI.ChangeWidget(:menu_bar, :EnabledItems,
        {
          :save  => true,
          :cut   => false,
          :paste => false
        }
      )

All other items in the menus will remain untouched, i.e. they will keep their current enabled / disabled status.

Comprehensive Example

See yast-ycp-ui-bindings/examples/MenuBar1.rb:

(Click the arrow to open)

# encoding: utf-8

module Yast
  class MenuBar1Client < Client
    Yast.import "UI"
    include Yast::Logger

    def main
      UI.OpenDialog(dialog_widgets)
      update_actions
      handle_events
      UI.CloseDialog
      nil
    end

    def dialog_widgets
      MinSize( 50, 20,
        VBox(
          MenuBar(Id(:menu_bar), main_menus),
          HVCenter(
            HVSquash(
              VBox(
                Left(Label("Last Event:")),
                VSpacing( 0.2 ),
                MinWidth( 20,
                  Label(Id(:last_event), Opt(:outputField), "<none>")
                ),
                VSpacing( 2 ),
                CheckBox(Id(:read_only), Opt(:notify), "Read &Only", true)
              )
            )
          ),
          Right(PushButton(Id(:cancel), "&Quit"))
        )
      )
    end

    def main_menus
      [
        Menu("&File", file_menu),
        Menu("&Edit", edit_menu),
        Menu("&View", view_menu),
        Menu("&Options", options_menu),
        Menu("&Debug", debug_menu)
      ].freeze
    end

    def file_menu
      [
        Item(Id(:open), "&Open..."),
        Item(Id(:save), "&Save"),
        Item(Id(:save_as), "Save &As..."),
        Item("---"),
        Item(Id(:quit), "&Quit"),
      ].freeze
    end

    def edit_menu
      [
        Item(Id(:cut), "C&ut"),
        Item(Id(:copy), "&Copy"),
        Item(Id(:paste), "&Paste"),
      ].freeze
    end

    def view_menu
      [
        Item(Id(:view_normal), "&Normal"),
        Item(Id(:view_compact), "&Compact"),
        Item(Id(:view_detailed), "&Detailed"),
        Item("---"),
        term(:menu, "&Zoom", zoom_menu),
      ].freeze
    end

    def zoom_menu
      [
        Item(Id(:zoom_in), "Zoom &In" ),
        Item(Id(:zoom_out), "Zoom &Out" ),
        Item(Id(:zoom_default), "Zoom &Default" ),
      ].freeze
    end

    def options_menu
      [
        Item(Id(:settings), "&Settings..."),
      ].freeze
    end

    def debug_menu
      [
        Item(Id(:dump_status), "Dump enabled / disabled &status"),
        Item(Id(:dump_disabled), "Dump &disabled items")
      ].freeze
    end

    def handle_events
      while true
        id = UI.UserInput

        case id
        when :quit, :cancel # :cancel is WM_CLOSE
          break # leave event loop
        when :read_only
          update_actions
        when :dump_status
          dump_item_status
        when :dump_disabled
          dump_disabled_ids
        end

        show_event(id)
      end
      id
    end

    def show_event(id)
      UI.ChangeWidget(:last_event, :Value, id.to_s)
    end

    # Enable or disable menu items depending on the current content of the
    # "Read Only" check box.
    def update_actions
      read_only = UI.QueryWidget(:read_only, :Value)

      # Enable or disable individual menu entries (actions as well as submenus):
      #
      # Specify a hash of item IDs with a boolean indicating if it should be
      # enabled (true) or disabled (false). Menu items that are not in this hash will
      # not be touched, i.e. they retain their current enabled / disabled status.

      UI.ChangeWidget(:menu_bar, :EnabledItems,
        {
          :save  => !read_only,
          :cut   => !read_only,
          :paste => !read_only
        }
      )
    end

    # Dump the enabled / disabled states of the menu items to the log.
    def dump_item_status
      states = UI.QueryWidget(:menu_bar, :EnabledItems)
      log.info("Enabled/disabled: #{states}")
    end

    # Dump the IDs of all disabled menu items to the log.
    def dump_disabled_ids
      states = UI.QueryWidget(:menu_bar, :EnabledItems)
      states.reject! { |k, v| v }
      log.info("Disabled: #{states.keys}")
    end
  end
end

Yast::MenuBar1Client.new.main

Related PRs

libyui SO version bump PRs:

src/YMenuBar.h Outdated Show resolved Hide resolved
src/YMenuButton.cc Outdated Show resolved Hide resolved
src/YMenuWidget.h Outdated Show resolved Hide resolved
src/YMenuWidget.cc Outdated Show resolved Hide resolved
Copy link

@wfeldt wfeldt left a comment

Choose a reason for hiding this comment

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

Not an expert here, but looks incredibly sensible to me.

src/YMenuBar.cc Show resolved Hide resolved
src/YWidgetFactory.cc Show resolved Hide resolved
@Conan-Kudo
Copy link

Conan-Kudo commented Aug 17, 2020

Wait, did you just reimplement the YMGAMenuBar widget without completely replacing it properly, or even pulling in the implementation from libyui-mga?

@shundhammer
Copy link
Contributor Author

See also manatools/libyui-mga#1

@shundhammer
Copy link
Contributor Author

See also libyui/libyui-gtk#73

@Conan-Kudo
Copy link

@shundhammer This is literally the worst way you could have handled this. You folks were definitely aware of YMGAMenuBar since I told all of you in the YaST meeting last month. The least you could do is implement it for libyui-gtk yourself.

@shundhammer
Copy link
Contributor Author

If you don't like it, don't use it; very simple.

And no, we don't do Gtk.

@anaselli
Copy link
Contributor

I will say mine, the intention is always to be constructive and not polemical.
In libyui mga issue you wrote

Once that is done, we might want to unify both widgets. Or if you prefer that, you can of course continue to use your MGAMenuBar.

My main intention is to use libyui and not reinvent the wheel. libyui-mga is meant to collect missing and/or common manatools widgets. That means that if something is missing -in past it has not been easy to add things in libyui-master- we try to add ourselves in MGA plugin.
We can consider the checkable table for instance, discarded by libyui because not needed, but used in most manatools applications.

Some mounth ago, people complained a lot the lack of menubar in dnfdragorta and i decided to try to develop it in mga plugin to avoid touching libyui objects. and with some ncurses exceptions works pretty well. The odd thing is that libyui implemented it some days after i finished...
So I'd expected you to ask for changes our branch to move it on libyui, that would have helped a lot since our apps would have moved in the same direction and the required changes would have helped me to understand how you want things in libyui, maybe in future branching there instead.
That would also had helped to have gtk already ready to use -it does not seem to be clear but most of dnfdragora issues are open against libyui-gtk users-, but as you say now we don't have and also you don't want to manage it. Again libyui is not our core and we do things for fun and free... I have a job and a real life and the approach i see here is not so community driven.... I'm sorry but that is my view since years, even if i'm happy that we can talk again in these last months...

What you're asking now is to study libyui changes and adding gtk in again, doubling my works instead....

Repeat again your last words here:

you can of course continue to use your MGAMenuBar.

Also that is pretty a lie, of course i can, but only because @Conan-Kudo and I can patch mageia and fedora packages, since libyui-binding issue 35 is still open since 29th Jun, much before new menubar widget came up in libyui.

@ancorgs
Copy link

ancorgs commented Aug 26, 2020

I will say mine, the intention is always to be constructive and not polemical.

Thanks a lot for that. So let's all be constructive and see how we can move forward.

In libyui mga issue you wrote

Once that is done, we might want to unify both widgets. Or if you prefer that, you can of course continue to use your MGAMenuBar.

Note the "once that is done" part. The new MenuBar widget should not be considered as 100% finished. This is just the preliminary version we will likely refine in the following weeks based on the YaST requisites (we are starting to use it for real). As our own resources permit, we would also like to take into account the manatools requisites (as long as it's reasonable for us to tackle them).

My main intention is to use libyui and not reinvent the wheel. libyui-mga is meant to collect missing and/or common manatools widgets. That means that if something is missing -in past it has not been easy to add things in libyui-master- we try to add ourselves in MGA plugin.

Fully understood. That's why @shundhammer tried to take the Gtk backend into consideration (eg. providing a default YWidgetFactory::createMenuBar() instead of making it pure virtual), even if we don't maintain anything Gtk-related anymore (since years).

Some mounth ago, people complained a lot the lack of menubar in dnfdragorta and i decided to try to develop it in mga plugin to avoid touching libyui objects. and with some ncurses exceptions works pretty well. The odd thing is that libyui implemented it some days after i finished...

@shundhammer indeed took a look to your work before starting the new MenuBar implementation. But the way we use libyui in YaST (for example, we don't call libyui directly but through a YCPValue-based UI interpreter) made it more practical for us to use a different API than the one in MGAMenuBar and, in general, to start from scratch.

So I'd expected you to ask for changes our branch to move it on libyui, that would have helped a lot since our apps would have moved in the same direction and the required changes would have helped me to understand how you want things in libyui, maybe in future branching there instead.

Understood.

That would also had helped to have gtk already ready to use -it does not seem to be clear but most of dnfdragora issues are open against libyui-gtk users-, but as you say now we don't have and also you don't want to manage it. Again libyui is not our core and we do things for fun and free... I have a job and a real life and the approach i see here is not so community driven.... I'm sorry but that is my view since years, even if i'm happy that we can talk again in these last months...

What you're asking now is to study libyui changes and adding gtk in again, doubling my works instead....

In the way MenuBar is written, we really hope that creating a Gtk backend for it would be simple enough, likely reusing a lot of knowledge (and code) from the Gtk backend of MGAMenuBar.

Repeat again your last words here:

you can of course continue to use your MGAMenuBar.

Also that is pretty a lie, of course i can, but only because @Conan-Kudo and I can patch mageia and fedora packages, since libyui-binding issue 35 is still open since 29th Jun, much before new menubar widget came up in libyui.

I don't see how the new MenuBar class makes this worse.

@anaselli
Copy link
Contributor

I don't see how the new MenuBar class makes this worse.

I didn't mean that, just that manatools is based on libyui-bindings for the most, first for perl, now more for python. So to use MGA plugin with new menubar without patching packagers, we also need that PR approved.
And yes that does not change things, since to use new libyui implementation we need a change in bindings too for new files added.

In the way MenuBar is written, we really hope that creating a Gtk backend for it would be simple enough, likely reusing a lot of knowledge (and code) from the Gtk backend of MGAMenuBar.

I think that too, but that does not change the fact I (or maybe someone else) need to do things twice.... :p

@anaselli
Copy link
Contributor

anaselli commented Aug 26, 2020

Fully understood. That's why @shundhammer tried to take the Gtk backend into consideration (eg. providing a default YWidgetFactory::createMenuBar() instead of making it pure virtual), even if we don't maintain anything Gtk-related anymore (since years).

hmmm i don't think it is enough... but at least builds :D

./MenuBar1 --gtk
Errore di segmentazione (core dump creato)

@shundhammer
Copy link
Contributor Author

For a more exhaustive example, see yast/yast-storage-ng#1122

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.

None yet

6 participants