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

Fix map background dialog width #3869

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Mar 16, 2023

Identify the Bug or Feature request

Fixes #3684. and fixes #755, #1157 some more.

Description of the Change

The main thing to change was to stop hardcoding a preferred size for the PaintChooser in the Choose Background / Chose Fog dialogs. This fixed the width of those dialogs, though the height of the dialogs suddenly became very large. The culprit there is the scrollable ImagePanel setting its preferred viewport size based on the number of rows it expects, which can be very many. I've solved this by taking a cue from JList : ImagePanel now has a configurable "visible row count" that is used to determine the preferred viewport height separately from the preferred height.

The Select Map Image dialog also had a hardcoded size that is now removed.


It became clear while working on this that both #755 and #1157 still existed, and I've included fixes for them as they are related. The problem was two discrepancies between ImagePanel.getPreferredSize() and ImagePanel.paintComponent():

  • for Scroll bar not always showing when needed in Image Pane of Resource Library #755, the problem is that paintComponent() sometimes renders one fewer columns than it should. Fewer columns means more rows, which causes the render to go beyond what getPreferredSize() was expecting vertically. Since getPreferredSize() is used in decide whether scrolling is need, this resulted in the apparenty issue of scrolling not being enabled when it should be.
  • for Resource Library's image pane allows too much vertical scrolling #1157, paintComponent() incorrectly calculated its vertical step by omitting the caption padding. All other height calculations included it, in particular getPreferredSize(). This caused the rendered height to be far less than the preferred height when there are many rows of thumbnails. As a result, scrolling was allowed to go far beyond the end of the thumbnail list.

The solution to both of these was to bring getPreferredSize() and paintComponent() more in line with each other in terms of layout. There are new methods for consistently calculating the real estate needed for a thumbnail (getItemWidth(), getItemHeight()), and a new method for calculating how many columns there should be (calculateItemsPerRow()). paintComponent() no longer does line wrapping, but instead calculates the current row and column using the above methods, making it consistent with getPreferredSize().

Possible Drawbacks

The sizing changes might not be optimized for different screen configurations.

Documentation Notes

N/A, bugfix

Release Notes

  • Fixed a bug where the Select Background and Select Fog dialogs were too small for their contents.
  • Fixed a bug where the Library and other windows would sometimes not allow scrolling even when the images overflowed.
  • Fixed a bug where the Library and other windows with lots of images would allow scrolling well past the end of the images.

This change is Reviewable

The `PaintChooser` and `MapSelectorDialog` no longer have explicit sizes set for them, but instead rely on their layout
to calculate a preferred size. `ImagePanel` needed to be updated to support this, by preferring viewports that might be
smaller than the preferred size of the panel itself.
Copy link
Contributor

@Phergus Phergus left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @kwvanderlinde)

@Phergus Phergus merged commit d8c5c8a into RPTools:develop Mar 16, 2023
@kwvanderlinde kwvanderlinde deleted the bugfix/755+1157+3684-fix-map-background-dialog-size branch March 16, 2023 23:50
@cwisniew cwisniew added the bug label Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Merged
4 participants