ppizarror / pygame-menu

A menu for pygame. Simple, and easy to use
https://pygame-menu.readthedocs.io/
Other
555 stars 141 forks source link

Vfill widget #403

Closed ppizarror closed 2 years ago

ppizarror commented 2 years ago
codecov[bot] commented 2 years ago

Codecov Report

Merging #403 (1678b13) into master (9c09143) will increase coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #403      +/-   ##
==========================================
+ Coverage   97.23%   97.24%   +0.01%     
==========================================
  Files          48       49       +1     
  Lines       12104    12155      +51     
==========================================
+ Hits        11769    11820      +51     
  Misses        335      335              
Impacted Files Coverage Δ
pygame_menu/__init__.py 94.87% <ø> (ø)
pygame_menu/widgets/__init__.py 100.00% <ø> (ø)
pygame_menu/widgets/widget/none.py 100.00% <ø> (ø)
pygame_menu/_widgetmanager.py 99.00% <100.00%> (+<0.01%) :arrow_up:
pygame_menu/menu.py 96.58% <100.00%> (+<0.01%) :arrow_up:
pygame_menu/widgets/core/widget.py 97.76% <100.00%> (+0.01%) :arrow_up:
pygame_menu/widgets/widget/__init__.py 100.00% <100.00%> (ø)
pygame_menu/widgets/widget/hmargin.py 100.00% <100.00%> (ø)
pygame_menu/widgets/widget/vfill.py 100.00% <100.00%> (ø)
pygame_menu/widgets/widget/vmargin.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9c09143...1678b13. Read the comment docs.

vnmabus commented 2 years ago

Can you merge the master into this branch, so I can try the code with the border size improvements?

ppizarror commented 2 years ago

Can you merge the master into this branch, so I can try the code with the border size improvements?

Done 💯

vnmabus commented 2 years ago

It seems that the width of VFill is the same as the width of the original menu. In my case, I adjust the width of the menu to the size of the widgets after they are all created. This worked before no matter what the initial size of the menu was, but if I add VFill widgets, I have to set the original width to 1 (0 is not allowed), otherwise the original width is kept.

I think that vertical alignment widgets should always have 0 width (or 1 if that is not supported). I am not sure if that affects other parts of the library.

ppizarror commented 2 years ago

Are you resizing the widgets? Because in tests, the following (without any further change) is true:

menu = MenuUtils.generic_menu()
b = menu.add.button('nice')
vf1 = menu.add.vertical_fill()
self.assertEqual(vf1.get_width(), 0)
vnmabus commented 2 years ago

Hum, if I print the size of each widget I obtain:

0
196
0
222
0
222
0
222
0
222
0
196
0
248
0
170
0

where the 0's are the VFill widgets, as expected.

However, menu.get_size(widget=True) returns (625, 717)! How can that be?

ppizarror commented 2 years ago

Can you provide a minimal working example? The following works as expected, thus, I cannot reproduce your issue:

menu = MenuUtils.generic_menu()
b = menu.add.button('nice')  # Add button
self.assertEqual(menu.get_size(widget=True), b.get_size())

# Now add 1 vfill, this should use all available height
vf1 = menu.add.vertical_fill()
self.assertEqual(vf1.get_width(), 0)
self.assertEqual(menu.get_size(widget=True), (b.get_width(), b.get_height() + vf1.get_height()))
ppizarror commented 2 years ago

Hum, if I print the size of each widget I obtain:

0
196
0
222
0
222
0
222
0
222
0
196
0
248
0
170
0

where the 0's are the VFill widgets, as expected.

However, menu.get_size(widget=True) returns (625, 717)! How can that be?

@vnmabus are you using a column/row layout?

vnmabus commented 2 years ago

@vnmabus are you using a column/row layout?

Nope.

ppizarror commented 2 years ago

@vnmabus are you using a column/row layout?

Nope.

Then I need an example. Maybe this widget introduces a bug, or you have spotted a new bug

vnmabus commented 2 years ago

The problem DOES NOT OCCUR if I remove widget_alignment=pygame_menu.locals.ALIGN_LEFT from the theme. I hope this clarify something for you.

vnmabus commented 2 years ago

So, this is a minimal working example:

import pygame
import pygame_menu

pygame.init()
surface = pygame.display.set_mode((800, 600))

theme = pygame_menu.Theme(
    widget_alignment=pygame_menu.locals.ALIGN_LEFT,
)

menu = pygame_menu.Menu(
    "",
    800,
    600,
    center_content=True,
    theme=theme,
)
# Now add 1 vfill, this should use all available height
vf1 = menu.add.vertical_fill()
print(menu.get_size(widget=True))

b = menu.add.button('nice')  # Add button
print(menu.get_size(widget=True), b.get_size())

# Now add 1 vfill, this should use all available height
vf2 = menu.add.vertical_fill()
print(menu.get_size(widget=True), b.get_size())
ppizarror commented 2 years ago

@vnmabus I've fixed the issue (and added the tests!). The problem was I overwrote NoneWidget.set_alignment, so VFill (and others) had an incorrect alignment; thus, when calculating the maximum and minimum x position, it assumed it was centered and used the whole container.

Let me know your thoughts 😎 . Greetings!

vnmabus commented 2 years ago

This fixes it, thanks!!

ppizarror commented 2 years ago

This fixes it, thanks!!

I'll merge to master then. Let me know about anything else. If you need, I can publish a new PyPI version

vnmabus commented 2 years ago

Can you publish in PyPI? I have to make a PR and need to upgrade the dependency to your package.

ppizarror commented 2 years ago

Can you publish in PyPI? I have to make a PR and need to upgrade the dependency to your package.

Done! See https://github.com/ppizarror/pygame-menu/releases/tag/4.2.7, https://pypi.org/project/pygame-menu/