Add UIForm Element

[GimLala]

Auto resizing containers added (Documentation on website is remaining)

These containers automatically resize when elements are added to them.

I was having some problems running tests, but I had tested them separately in a project I was using them in. More similar elements are coming like UIAutoScrollingContainer which uses an auto resizing container as its scrollable container instead of a normal UIContainer.
Check if you like this implementation, or maybe we could even merge UIContainer and the new UIAutoResizingContainer

Adding PR split list to first post:

Easy changes

1. Adding .idea to .gitignore PR - Add .idea to gitignore #530

Changes to existing base classes

1. Changes being made to UIElement - This needs some merge effort with other recent changes, I had a stab locally but I think it can probably be done better. Also I need to interrogate what theses change are more fully as changes here can have a wide impact across the codebase as I discovered last week with a smaller change to dynamic resizing, that passed all the unit tests, managing to break an image resizing demo. Tidy UIElement typehinting #531, Add set & get methods for ui element anchors #534, Make offset positioning calculation functions static #535, Refactor update abs rect pos to use static calc func #536
2. Changes to UIContainer - Same reasoning as above regarding wide ranging impact. Add expand_left & _top to UIContainer #537
3. Changes to UIScrollingContainer - I'm less concerned about changes here as it is not widely used, but it's worth looking at - there is also the possibility we may want to merge this class entirely with AutoScrollingContainer. Tidy UIScrollingContainer type hinting #538, Add axis constraints to UIScrollingContainer #539

New stuff

1. AutoScrollingContainer Add Auto Scrolling Container #545
2. AutoResizingContainer Add Auto-resizing container #543, Fix auto resizing container #544
3. UI2DSlider Add UI2DSlider element #540
4. UIForm

Using the new stuff in existing stuff

1. UIColourPicker addition of UI2DSlider Add 2D slider to colour picker window #542

[MyreMylar]

The main question I had when looking over this is - are there circumstances where you would want a container that resizes like this, as opposed to one that resizes it's scrollable area (e.g. the way an explorer or one where you have decided the size ahead of time to fit the stuff you are going to put inside of it? Most UIs I see these days adopt a 'squish and vertical scroll' for oversized content - I suspect mainly because of the dominance of phones and tablets in browsing content.

I suspect there are - but listing off some ideas as to what this would be used for in a UI would probably help clarify the design and help create an example that uses this type of container for the examples repository:

https://github.com/MyreMylar/pygame_gui_examples

[GimLala]

I'll admit that I didn't have too much in mind except it would be easy to replace the UIContainer used in the UIScrollableContainer to this special container. I thought we would have to implement something like this either straight into the UIScrollableContainer otherwise anyways, and implementing it this way would potentially allow for more use cases to be available with the same effort for maintenance.

[GimLala]

I think the usage possibility would be when you'd require a container to extend to contained elements but not to the point where you'd need scrollbars, by restricting the maximum_edges rect appropriately.
So for example when you want to put a UIContainer inside a UIScrollableContainer, you wouldn't want the inner container to have scrollbars regardless of how large it becomes because then you'd end up having a container with scrollbars inside a container with scrollbars.
I understand that this implementation doesn't have a ton of more added freedom of usage, but it still adds at least some more freedom to use this feature and I don't see any major downsides to it

[MyreMylar]

Ok, I've finally had a chance to look over this. Apologies for the delay.

I noticed while testing this that something in the rendering machinery is not ready for:

test_resizing_container = UIAutoResizingContainer(pygame.Rect(0, 0, 50, 50))
hello_button = UIButton((60, 0), 'Hello', container=test_resizing_container)

It seems to get negative dimensions passed down to it and is unhappy, so that is something to look into further.

The on_content_moved_or_resized() function I suggested for the UIContainer may also be useful when trying to use this in the UIScrollingContainer as we will need to trigger the code that is currently in the UIScrollingContainer's set_scrollable_area_dimensions() function somehow, perhaps we can have it set a flag variable 'this container has resizedthat theUIScrollingContainer` can check.

Anyway, thanks for your hard work on this. I think it is getting somewhere useful 🎉

[GimLala]

Hi,
Thank you for reviewing the pull request. I also found some flaws with this code when resizing the container. I think there are certain other major problems with the class which I am trying to pin point properly.
However, I had already finished most work on the uiscrolling container which automatically resizes, and some other elements.
Yet I cannot guarantee that I will have the time to fix all the issues as I have some important exams coming up which will last the whole of February. I will try my best to debug the problems and update the necessary code, but I think it might be better to have the other elements' code added. Currently, I have these elements (almost) finished:

• UIAutoScrollingContainer
• UI2DSlider
• UIColourPickerDialog (with an added 2Dslider in the colour square which allows dragging to select the correct colour)
• UIForm

These elements aren't fully completed, and I'm sure there are bugs but you'll be able to see what my approach to those elements is and guide me if necessary. You may even finish it if I am too inactive. It will atleast get the process of adding those elements going

[GimLala]

While renaming the UIContainer method on_anchor_target_changed to on_content_moved_or_resized as you suggested, I saw that the UIElement class has a method called _on_content_changed which has no documentation. What exactly does this method do? Perhaps some documentation with that would make it unambiguous about which function does what.

Secondly, the get_top_most_point is not necessarily the same as getting the self.rect.top, because self.rect.top will not be changed to the y value from get_top_most_point unless resize_top is True. This means that using this method could be way to check what size the container will be in the case where resize_top is False and they wish to toggle it to True.
However, I can add the prefix if you feel like that use case is too specific and may confuse the users.

[GimLala]

I made some changes using your suggestions. However, I can just not get the container to work properly. It seems to be having some problems with buttons and their texts, and especially with dynamic width and heights.

[GimLala]

I think I found the root of the problem. So what happens when we create a new button with dynamic width and height is:

1. First, it creates a full sized image containing the text.
2. Then it sees how much of the image fits in the container, and crops the image accordingly.
3. However, it also resizes its rect to the size of the image.
4. This means that since the AutoResizingContainer checks whether it needs to expand using the rect of the button, it fails to recognise that the button exceeds the borders.
5. Now since the container doesn't resize, no matter how many times you try to resize the button, the button will always be invisible since the rect doesn't change, the clipping rect doesn't change and the image size doesn't change.

So, to fix this problem, we'll have to stop the image from resizing based on if it's outside the container. Just update the clipping rect instead and as soon as the element moves update the clipping rect, and only blit the the image actually contained within the container.

[GimLala]

I finally was able to fix the problem by eleminating the if condition in UIElement._set_image which checks whether the image_clipping_rect is 0 and sets the universal_emtpy_surface as the self.image.

[MyreMylar]

Looks like a crop of the unit tests are failing now I've enabled them to run.

[GimLala]

Looks like a crop of the unit tests are failing now I've enabled them to run.

I have tried fixing some of the failing tests with the next commit, could you try re-running the workflow? it doesn't allow me to run it myself. Also, when I run the tests in my IDE, many of them fail because the manager can't find the specified files, or the colours of the gradient don't match, etc. I don't think I have changed any code which would interfere with the colours or location of files. There is a test which specifically tests for the if condition I had removed, where in the expected image was supposed to be the ui_manager.universal_empty_surface but after I changed it (so that the size of the image isn't 0, 0) the tests fails.

I will also add the appropriate tests in a few days

[MyreMylar]

Looks like a crop of the unit tests are failing now I've enabled them to run.

I have tried fixing some of the failing tests with the next commit, could you try re-running the workflow? it doesn't allow me to run it myself. Also, when I run the tests in my IDE, many of them fail because the manager can't find the specified files, or the colours of the gradient don't match, etc. I don't think I have changed any code which would interfere with the colours or location of files. There is a test which specifically tests for the if condition I had removed, where in the expected image was supposed to be the ui_manager.universal_empty_surface but after I changed it (so that the size of the image isn't 0, 0) the tests fails.

I will also add the appropriate tests in a few days

I have kicked off the tests.

To run them locally you should just be able to run pytest tests from the root directory (the one that has the 'tests', 'docs' and 'pygame_gui' folders. For me it looks like this:

[GimLala]

The tests are failing currently, because of a singular line which updates the dimensions of the UIAutoResizingContainer within the UIScrollingContainer when the absolute min and max rects are calculated.
The tests worked before because they relied on the scroll_bars appearing when the scrollable area is increased beyond the size of the view container, but because of that line, it automatically detects that it should not be the increased size and resizes back down to the size of the min_edges_rect, which is just the original size passed in.
This means that the set_scrollable_area_dimensions is useless when should_grow_automatically is set to True, which may cause some bugs with previously working applications?

1. Should we make it so that when the set_scrollable_area_dimensions is used, we should turn should_grow_automatically to False? and/or
2. Should we make it so that when set_dimensions is called on a UIAutoResizingContainer, we also set should_update_dimensions to True?

[GimLala]

Also, should we raise a warnings for certain methods which UIForm inherits from UIScrollingContainer, but aren't intended to be used with UIForm? for example, set_scrollable_area_dimensions

[GimLala]

As you can see from the commit above, I started adding the tests to UIForm. You can see a list of todo comments which lists what tests are left to add/complete. Let me know if any tests are needed apart from that

[GimLala]

The code cov doesn't seem to be running after a whole day

Edit: Now it does after the new commit, code cov seems to be slightly buggy

[GimLala]

Some tests seem to be failing when we put a UIForm inside a UIForm, but I suspect it has something to do with UIScrollingContainers instead. I'll remove the UIForms within UIForms, but this should be something to look into.
But surprisingly, the tests didn't fail when I hadn't added a section within the form

[MyreMylar]

The tests are failing currently, because of a singular line which updates the dimensions of the UIAutoResizingContainer within the UIScrollingContainer when the absolute min and max rects are calculated.
The tests worked before because they relied on the scroll_bars appearing when the scrollable area is increased beyond the size of the view container, but because of that line, it automatically detects that it should not be the increased size and resizes back down to the size of the min_edges_rect, which is just the original size passed in.
This means that the set_scrollable_area_dimensions is useless when should_grow_automatically is set to True, which may cause some bugs with previously working applications?

Should we make it so that when the set_scrollable_area_dimensions is used, we should turn should_grow_automatically to False? and/or
Should we make it so that when set_dimensions is called on a UIAutoResizingContainer, we also set should_update_dimensions to True?

Might be late to the party on replying to this. I guess the spirit of the original auto scrolling container was that it would grow & shrink based on the contents of the auto resizing container - meanwhile the set_scrolling_dimensions function sort of ignores that and just sets a size for the scrolling contents.

I think using set_scrolling_dimensions should probably disable the auto growing and shrinking aspect of the auto resizing container - until it is explicitly turned back on (which should also be an option). But we should also update the documentation of the function to say that.

Also, should we raise a warnings for certain methods which UIForm inherits from UIScrollingContainer, but aren't intended to be used with UIForm? for example, set_scrollable_area_dimensions

Various ways to handle this:

a) make UIForm have a "has a" relationship to the UIScrolling container rather than an "is a" relationship.
b) override "set_scrollable_area_dimensions" for the UIForm so it doesn't call the base version in UIScrollingContainer and does nothing except issue a warning.
c) make it work OK with the UIForm even if it is a bit dumb?

Edit: Now it does after the new commit, code cov seems to be slightly buggy

Yeah, it sometimes seems to skip an update to a pull request, I've not figured out exactly why, but it is better than nothing!

[GimLala]

I think using set_scrolling_dimensions should probably disable the auto growing and shrinking aspect of the auto resizing container - until it is explicitly turned back on (which should also be an option). But we should also update the documentation of the function to say that.

I agree with that. We can just add a getter and setter for the auto growing option. Alternatively, we could also add the option to pass in -1, -1 in set_scrollable_dimensions to turn it on.

a) make UIForm have a "has a" relationship to the UIScrolling container rather than an "is a" relationship.

I think I agree with this the least because it doesn't feel right to have to use 2 UIElements to do the work of 1. Also, it makes sense to me that UIForm is a scrolling container, rather than having one. We'd also have to override some functions to call the methods of the contained scrolling container anyway, and that is just extra work for no reward.
It feels similar to saying whether each sub-class of UIElement could just instead contain a UIElement instance instead of being one itself and just have all of the methods operate on that UIElement instance instead. Sure, it would work, but it makes things more confusing and unintuitive and would definitely lead to unexpected bugs in the future.

Infact, it did raise confusion for me when UISelectionList was just inheriting from UIElement instead of UIContainer or some other class because it clearly does contain other elements. Also, it has a list_and_scroll_bar_container even though I think it should be the list_and_scroll_bar_container. Might be something to look at in the future, perhaps.

b) override "set_scrollable_area_dimensions" for the UIForm so it doesn't call the base version in UIScrollingContainer and does nothing except issue a warning.

I feel like if someone was to call the function, it's their responsibility for what the function does. I think that would be the expected behaviour as well, instead of just blocking them from the usage of that method. The only such case where we should interfere is when calling the function breaks something about the element or crashing the program. If it is something untested, or something that we don't expect the user to use, or perhaps use a different method, then we should raise a warning, but still call the base method. If there is just one semi-reasonable way of doing something, even if we don't know why that thing would be needed, we don't need to raise a warning or block the way. We just need to try and make sure that the method does what its name suggests it should do.

So in conclusion, I think it should vary between b) and c) (or a mixture of the two) for different methods.

[GimLala]

Infact, it did raise confusion for me when UISelectionList was just inheriting from UIElement instead of UIContainer or some other class because it clearly does contain other elements. Also, it has a list_and_scroll_bar_container even though I think it should be the list_and_scroll_bar_container. Might be something to look at in the future, perhaps.

Also, UIScrollingContainer should be the _root_container. I don't see why we need a root container, because we anyways have to relay the set_position, set_relative_position and set_dimension calls to the root container anyway.
The fact that UIScrollingContainer doesn't even inherit from UIContainer or IUIContainerLikeInterface just feels wrong. Also, since both the UIScrollingContainer and the _root_container are within the parent container, it makes counting the number of elements more confusing.

Similar points for the UITabContainer not inheriting from UIContainer, IUIContainerLikeInterface or even IContainerLikeInterface and UIPanel, where almost every single method has to relay itself to the panel_container, although it doesn't look as wrong for UIPanel to inherit from IContainerLikeInterface instead of UIContainer or IUIContainerLikeInterface (even though it should inherit from UIContainer)

But I do understand UIWindow having a container instead of being a container. It does feel like it has a lot more to it than just being a container, although it could still just have been a UIContainer instead of having a window_root_container. But I don't think it's nearly bad enough to need to change

[GimLala]

I was checking the recursion errors and one of them happens when the size is set to 50 or lesser in the horizontal axis or 5 or lesser in the vertical axis. I don't understand why though, and there are more errors to find which happen when UIForms are placed within UIForms.

[GimLala]

I think I will not have much time to work on this to complete the remaining tests unfortunately. I hope I have made sufficient enough progress that you can finish the remaining tests as I have highlighted in some TODO comments at the top of the test class.
however, I may be able to assist if there are weird errors raised by this element.
P.S. Have you seen my comments above?

[MyreMylar]

I think we will try merging this and see how people get on with it.

I don't personally have any use cases for it in my existing games or demos so I'm just adding the test program @GimLala posted to the examples repository,