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

Allow drawing dropdown lists with scrollbars above the widgets #6060

Closed
DorpsGek opened this issue Jul 12, 2014 · 7 comments
Closed

Allow drawing dropdown lists with scrollbars above the widgets #6060

DorpsGek opened this issue Jul 12, 2014 · 7 comments
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

juanjo opened the ticket and wrote:

Currently dropdown lists are put above or below the widget:

  1. If there is enough space below the widget that opens the list, draw it below.
  2. Else, if there is enough space above the widget, draw it above.
  3. Finally, if the complete list cannot be drawn because there isn't enough space, draw it below with a scrollbar.

Moreover, if 1 and 2 fails, and there is not enough space to draw at least one item below the widget, the next assert is triggered (in a not-too-close point of the code):
Assertion failed at line 688 of .../src/widgets/../widget_type.h: capacity > 0

The patch tries to change the procedure for choosing a place to draw the list:

  1. If there is enough space below the widget that opens the list, draw it below.
    2b) Else, check where is more space available and see if a scrollbar is needed.
    3b) Before continuing, if there is no space for drawing a single item, stop with an assert(). (Could be changed to show an alert or debug message or to return without drawing the list).

This way, it is possible to draw a dropdown list with a scrollbar above the widget. In addition, if the list cannot be shown, an assert is triggered as well, but in a place where it is easier to understand the problem.

There are several changes on names of variables:

+int available_height = GetMainViewBottom() - top - 4;
so the expression "top + height + 4 >= GetMainViewBottom()" becomes "height >= available_height".

+int available_height_above = w->top + wi_rect.top - GetMainViewTop() - 4;
so the expression "w->top + wi_rect.top - height > GetMainViewTop()" becomes "available_height_above > height".

The expression "screen_bottom - 4 - top" becomes "available_height".

Attachments

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/6060
@DorpsGek
Copy link
Member Author

juanjo wrote:

The task is a patch, not a bug...


This comment was imported from FlySpray: https://bugs.openttd.org/task/6060#comment13397

@DorpsGek
Copy link
Member Author

frosch wrote:

It's funny how the dropdown starts scrolling downwards when opening it and holding down the mouse button. I wasn't even aware that it scrolled at borders like that.

Maybe it can be fooled by opening the dropdown scrolled to the very bottom, when opening it towards the top.


This comment was imported from FlySpray: https://bugs.openttd.org/task/6060#comment13400

@DorpsGek
Copy link
Member Author

juanjo wrote:

I added 4 final lines to do as frosch said. The trick is the same used when opening the dropdown with scrollbar below the widget, which opened the dropdown scrolled to the top position by default.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6060#comment13404

@DorpsGek
Copy link
Member Author

DorpsGek commented Apr 2, 2017

juanjo wrote:

With the recent changes in r27820, if a dropdown is opened above the parent widget and with a scrollbar, as frosch commented, it starts scrolling downwards.
The next patch can be applied to r27837.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/6060#comment14406

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 7, 2018
@frosch123 frosch123 removed the Core label Apr 14, 2018
@andythenorth andythenorth added the stale Stale issues label Jan 5, 2019
@andythenorth
Copy link
Contributor

andythenorth commented Jan 12, 2019

Patch from 2 April 2017 applies and compiles cleanly

I have no idea how to test it though :) The original assert issue I can repro in r27656, but is gone in 1.8.0. Not sure what the remaining patch does so can't confirm it works or not.

Edit: frosch explained to me how to trigger the bug with unexpected scrolling ^ but I couldn't repro it. Further investigation produced:

frosch123: so, the scrolling still happens before the ontick stuff, and probably again with #7048

@J0anJosep interested in gravedigging this one?

Thanks!

@andythenorth
Copy link
Contributor

andythenorth commented Jan 12, 2019

#7048 is now merged. I can repro the unexpected scroll, and the patch from 2 April appears to fix it. Needs a PR, but I won't do that tonight :)

@andythenorth
Copy link
Contributor

JJ indicates here that this is being worked on further: #7052

I'm cleaning up the labels for this one.

@andythenorth andythenorth removed the stale Stale issues label Jan 13, 2019
J0anJosep added a commit to J0anJosep/OpenTTD that referenced this issue Jan 13, 2019
J0anJosep added a commit to J0anJosep/OpenTTD that referenced this issue Jan 13, 2019
…scroll it to its very bottom.

This "prevents" the fast movement towards the bottom when holding down the mouse button.
J0anJosep added a commit to J0anJosep/OpenTTD that referenced this issue Jan 14, 2019
J0anJosep added a commit to J0anJosep/OpenTTD that referenced this issue Jan 14, 2019
…scroll it to its very bottom.

This "prevents" the fast movement towards the bottom when holding down the mouse button.
@PeterN PeterN closed this as completed in 628af2f Jan 20, 2019
nielsmh pushed a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
nielsmh pushed a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
…scroll it to its very bottom.

This "prevents" the fast movement towards the bottom when holding down the mouse button.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Development

No branches or pull requests

3 participants