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. [x] Adding .idea to .gitignore PR - #530

Changes to existing base classes

1. [x] 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. #531, #534, #535, #536
2. [x] Changes to UIContainer - Same reasoning as above regarding wide ranging impact. #537
3. [x] 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. #538, #539

New stuff

1. [x] AutoScrollingContainer #545
2. [x] AutoResizingContainer #543, #544
3. [x] UI2DSlider #540
4. [ ] UIForm

Using the new stuff in existing stuff

1. [x] UIColourPicker addition of UI2DSlider #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

[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]

Okay, I have finally seemed to squash the bugs coming up with the containers while resizing. Also, I added some other functions and removed other bugs with this, namely:

1. UIContainer now also has expand_top and expand_left functions which work like set_dimensions except, as the name suggests, they expand the container from the top and left respectively. The big difference between just setting the dimensions and moving the rect, and these functions is that these functions also update the relative rects of the contained elements. This means that none of the contained elements' absolute positions will move.
2. I updated the on_contained_elements_changed method to recursively update targets as when there was a chain of elements all anchored to each other, it would not update all of them previously

I ran all the tests and it seems to be working properly. I tested the elements manually and they are working but I have not written the pytest equivalent tests yet. It's a bit daunting of a task because there are so many things to test. If you run the tests again they should all be successful

[MyreMylar]

Looks like things are coming along. These are the failing tests at the minute:

FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_creation - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_create_too_small - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_press_cancel_button - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_press_ok_button - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_click_in_saturation_value_square_button - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_mess_with_colour_channel_event - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_update_saturation_value_square - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_update_current_colour_image - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_changed_rgb_update_hsv - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_changed_hsv_update_rgb - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_show - AttributeError: 'UI2DSlider' object has no attribute 'sliding_button'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_hide - AttributeError: 'NoneType' object has no attribute 'width'
FAILED tests/test_windows/test_ui_colour_picker_dialog.py::TestUIColourPickerDialog::test_show_hide_rendering - AttributeError: 'UI2DSlider' object has no attribute 'sliding_button'

And most of them look like the same fail there.

[GimLala]

Ah, My "Fixed Potential Bug" commit is the culprit here. Added comments so that I don't unfix a bug again. However, 2 of the tests are still failling I'll see what is causing them

[GimLala]

The tests were failing mainly because the UIButton in the UI2DSlider kept consuming the event so the colour wouldn't change. Usually this wouldn't be a problem because clicking the button in place shouldn't change the colour anyway, but because this was done by the program where the colour was set programmatically and the necessary values not updated properly, it would cause errors

[GimLala]

Also, now there is a border around the sat square of the UIColourPickerDialog due to the 2DSlider. You can decide whether you want that or not and set the border to transparent in the default file if you don't want it

[GimLala]

One of the tests is failing but I can't figure out why. The changes I made really look right to me when I locally tested it, but I don't know which part of the code I changed is affecting the test and therefore whether that was by intention or not. Mysteriously, even though the test works with the horizontal scrollbar of the scrollable container, it doesn't with the vertical scrollbar

[GimLala]

Why are the two scrollbars implemented a bit differently? could be something to do with it. I think I need your help debugging with this one, @MyreMylar because I can't even figure out what the bug is and whether it affects the container at all to the user

[GimLala]

Aha! I have fixed the bugs. It had to do with the visible container being resized after calculations of the sizes of the scrollbars instead of before the calculations, as it was done before. I will start work on the tests, but some features remaining to add are:

1. Padding around auto resizing containers, because currently it is made so that it will fit perfectly to the elements inside clamped by its min and max edges rect, and being able to change that would help. but, I am not sure whether I should use the theming file or a value passed in during initialisation because I think other IUIContainer elements like UIAutoScrollingContainer and UIForm would like to use this functionality. If we use the theme file then I'm not sure how the other elements will use it.

2. Some more types of inputs in UIForm, like a colour input using colour pickers, or another type of field which are required to be input, and perhaps a password type input. I have created an issue, #526 for the second one. I have a way for the user of the library to create a questionnaire with required elements, but the actual presence check has not been implemented so it's currently not functional.

3. Validation of min and max edges rect in autoresizing containers to make sure that min edges rect is contained completely within max edges rect, otherwise raise a warning perhaps?

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 85.56876% with 85 lines in your changes are missing coverage. Please review.

Project coverage is 95.98%. Comparing base (7a5f9ef) to head (ae12b16). Report is 12 commits behind head on main.

Files	Patch %	Lines
pygame_gui/elements/ui_form.py	85.47%	85 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #494      +/-   ##
==========================================
- Coverage   96.27%   95.98%   -0.29%     
==========================================
  Files          89       90       +1     
  Lines       13217    13909     +692     
==========================================
+ Hits        12725    13351     +626     
- Misses        492      558      +66     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[MyreMylar]

Great looks like the current tests are compiling!

I will give this a good look over today, if its all good then we'll just need some new unit tests to cover the new elements.

[MyreMylar]

Absolutely, did not get onto looking at this today sorry, but it is on my to do list for this week.

[MyreMylar]

OK, I've got this running locally and browsed through bits of it - I have run into a few bugs along the way, most of which should get squished through coverage compliant unit testing - which I'll add some separate comments about as I go.

I think my biggest issue overall with this PR is its size is making it tough to mentally digest everything going on inside of it. Sadly I only have a small amount of time each week to work on coding projects at the moment which makes dumping this much code in my head at once very tough!

I'm going to suggest we break this down into about nine PRs and in the following order - like so:

Easy changes

1. [x] Adding .idea to .gitignore PR - #530

Changes to existing base classes

1. [x] 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. #531, #534, #535, #536
2. [x] Changes to UIContainer - Same reasoning as above regarding wide ranging impact. #537
3. [x] 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. #538, #539

New stuff

1. [x] AutoScrollingContainer #545
2. [x] AutoResizingContainer #543, #544
3. [x] UI2DSlider #540
4. [ ] UIForm

Using the new stuff in existing stuff

1. [x] UIColourPicker addition of UI2DSlider #542

Then we can make sure each step is properly looked at and tested before moving up the chain. I'll help with this task by creating PRs with the changes and adding you as co-author to the commits so you get credit. You can also join in this process if you like, not sure how much you've used git and multiple branches, but I'll start off with the first couple anyway.

[MyreMylar]

OK, I've broken down most of the changes to the core stuff now into new PRs and added tests (the remaining changes are ones I'm not sure are actually needed yet).

I'm going to split off the 4 new elements into separate PRs next.

[GimLala]

Hi, I have been out recently for vacation on a tour. I'll see what exactly are the changes when I get back in a few days. I realised the PR got very big very quickly so it's great that you're breaking it down. Some bugs I can tell were caused by short sighted changes by me failing to account for their impact, but they should be easily fixable.

[GimLala]

Hi, in the other separate PR's, have you made any changes to the code from this PR? If that's the case, then the only change needed will be to this PR to fix the errors you mentioned above (which I think I have fixed)

[MyreMylar]

Hi, in the other separate PR's, have you made any changes to the code from this PR? If that's the case, then the only change needed will be to this PR to fix the errors you mentioned above (which I think I have fixed)

If you look at the 'files changed' link at the top there you can see what stuff is going to get merged from this PR now.

Essentially the other PRs I made have stripped out and merged to main everything except the UIForm stuff which is now all that is left in this one.

Last time I looked at the UIForm I realised that the drop down really needs to support a dynamic width to work properly with something like this - where arbitrary data can be added to the drop down. I wanted to figure out a way to do that (it's tricky because drop downs start closed but you would want a dynamic width one to be wide enough to fit every item in the drop down list.

I think the form also needs a good looking example of a form creating in the pygame_gui_examples Github project as it is a complicated element and is likely to be vulnerable to changes in lots of other areas so I'd like to have something to run an check before making releases.

So basically I want to get those steps worked out before merging it. They may in turn throw up other things. I have a little list of things to look at over my next work holiday and getting this merged in is part of it - along with a new release of the library.

[MyreMylar]

I guess the form could also use a test file to give it a good work out and make the test coverage check pass.

[GimLala]

I saw that in one of the branch merges with main, you un-committed the changes from my "adding default 0 sized scrollbars" commit, which added those scrollbars to the UIScrollingContainer so that the elements in the UIForm could anchor to it. Is there a reason you did so? Because now the UIForm isn't working for me.

I think that it's better to add a scrollbar to the dropdown then make it dynamically sized, as it may not look great if it expanded to the length of the entire screen. Perhaps it would extend up to the length specified in the height limit, then add a scrollbar which allows you to scroll through the options.

I had a working example (although perhaps not good looking: I had no specific theming applied), which pretty much used all the features I have added. I can modify and add that in once we fix the issues with the default scrollbars.

I'll also create a test file (or add any missing test functions if you create one before me), but I will likely only have time from around 6th because I have a really busy week coming up.

[MyreMylar]

I saw that in one of the branch merges with main, you un-committed the changes from my "adding default 0 sized scrollbars" commit, which added those scrollbars to the UIScrollingContainer so that the elements in the UIForm could anchor to it. Is there a reason you did so? Because now the UIForm isn't working for me.

The scrolling container is made op of two containers - an 'outer' one that contains the 'inner' container and the scroll bar and an 'inner' container that contains the contents. The contents should be unaffected by whether there is a scrollbar in the outer container or not as they should only know about the inner container's dimensions.

If there is an issue with the resizing of the inner container and the effect on it's contents then we need to solve that bug not mess the scroll bar which should have no impact on the contents.

I think that it's better to add a scrollbar to the dropdown then make it dynamically sized, as it may not look great if it expanded to the length of the entire screen. Perhaps it would extend up to the length specified in the height limit, then add a scrollbar which allows you to scroll through the options.

The drop down already has a scroll bar for height, I was talking about dynamic sizing for width here (and I guess also font size height) not the number of items in a drop down.

I had a working example (although perhaps not good looking: I had no specific theming applied), which pretty much used all the features I have added. I can modify and add that in once we fix the issues with the default scrollbars.

I'll also create a test file (or add any missing test functions if you create one before me), but I will likely only have time from around 6th because I have a really busy week coming up.

Great! I will get back to this and give it a more thorough going over but any example usage code you have you can just slap into this issue in a code block so I can have look.