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 libyui Widget: YMenuBar #73

Closed
shundhammer opened this issue Aug 18, 2020 · 16 comments
Closed

New libyui Widget: YMenuBar #73

shundhammer opened this issue Aug 18, 2020 · 16 comments
Labels
other-maintainer Not maintained by the core YaST team

Comments

@shundhammer
Copy link
Contributor

(See also manatools/libyui-mga#1)

We added a MenuBar widget to libyui. More details here:

https://github.com/libyui/libyui/pull/169

C++ example:

https://github.com/libyui/libyui/blob/master/examples/MenuBar1.cc

The YWidgetFactory::createMenuBar() method right now has a default implementation that just returns a null pointer. Once The Gtk UI implements it, we intend to make it pure virtual in libyui just like the other factory methods.

anaselli added a commit to anaselli/libyui-gtk that referenced this issue Nov 1, 2020
@anaselli
Copy link
Contributor

anaselli commented Nov 1, 2020

I think it could be almost finished but please start reviewing it.

I will make more tests also to check if i can use it on our tools before asking for a PR.

@shundhammer
Copy link
Contributor Author

That commit looks good to me (but I am in no way a Gtk expert).

@anaselli
Copy link
Contributor

anaselli commented Nov 2, 2020

Ok i ported my YMGAMenuBar example and now it uses libyui YMenuBar.

Please test it.
I experienced a ncurses almost good behavior, but when hidden menu is shown that goes over the windows size:

immagine

Changing menu (pressing Change menu button) does not refresh new menu though, until on menu an action is taken (selection for instance).
Another issue is on separators, using ncurses that feature is missing:

immagine

The Qt one instead is not respecting the startup menu setup:

immagine

Gtk one seems to be ok:

immagine

@anaselli
Copy link
Contributor

anaselli commented Nov 3, 2020

In the hope it can help I add here some thoughts.

As far as i can say in a very quick look for Qt i think the problem is in YQMenuBar::rebuildMenuTree where enablel and visible item properties are not checked.

For ncurses separator i used w.hline(ACS_HLINE, 0); but i know there is a separator NCTableCol::SEPARATOR that maybe you are able to use, i wasn't. Concerning redrawing for new menu i haven't looked at the code, i think a redraw in the right place should fix it.

@shundhammer
Copy link
Contributor Author

As far as i can say in a very quick look for Qt i think the problem is in YQMenuBar::rebuildMenuTree where enabled and visible item properties are not checked.

You are right. @ancorgs noticed that indepentendly, and it's now fixed, and we also have a new test case:

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

Fixed with libyui/libyui-qt#139; new test case with yast/yast-ycp-ui-bindings#60 .

@shundhammer
Copy link
Contributor Author

Also fixed initial menu item visibility in the Qt UI.
@anaselli thanks for noticing!

@anaselli
Copy link
Contributor

anaselli commented Nov 3, 2020

I could ask for a PR then soon, and going on testing by using right issue channels maybe. I'll wait some days to see if the fixing is available on master. Thanks.

@anaselli
Copy link
Contributor

anaselli commented Nov 8, 2020

Also fixed initial menu item visibility in the Qt UI.
@anaselli thanks for noticing!

I pulled libyui-qt but i seem it still does not show initial hidden and disable items... am i wrong in something?
immagine

@shundhammer
Copy link
Contributor Author

I can see the commit for initial enabled/disabled state in libyui-qt master:

libyui/libyui-qt@f437e9c

also for initial visibility:

libyui/libyui-qt@5fdcc79 and libyui/libyui-qt@aa7f13d

Let me check this with your example https://gist.github.com/anaselli/f243a82ab1dda0898b1334f5386e12b8 when today's long sequence of meetings will finally be done.

@anaselli
Copy link
Contributor

anaselli commented Nov 9, 2020

sure :) I also asked for a PR ;)

@anaselli
Copy link
Contributor

anaselli commented Nov 9, 2020

hmm i think i've got it... I added an hidden menu, not an hidden menu item or submenu, that seems to be a missing case into the code

@shundhammer
Copy link
Contributor Author

Ah, okay; that might indeed be missing. But it shouldn't be too hard to add.

@shundhammer
Copy link
Contributor Author

This commit fixes initial visibility and enabled/disabled state for the Qt UI for toplevel menus:

libyui/libyui-qt@61594b6

This will become part of the next upcoming PR.

@anaselli
Copy link
Contributor

👍

@ancorgs
Copy link

ancorgs commented Mar 17, 2021

Since the Gtk backend is not developed or maintained by the YaST Team at SUSE, I'm adding the "other-maintainer" label to this in order to help filtering the various list of issues.

@ancorgs ancorgs added the other-maintainer Not maintained by the core YaST team label Mar 17, 2021
@anaselli
Copy link
Contributor

anaselli commented May 9, 2021

I think i reported here some behaviors that should change to ncurses... but as far as libyui-gtk concern i think the issue is fixed

@anaselli anaselli closed this as completed May 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
other-maintainer Not maintained by the core YaST team
Projects
None yet
Development

No branches or pull requests

3 participants