Ticket #3173 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

Popup dialogs wander upwards upon resize

Reported by: egmont Owned by: andrew_b
Priority: trivial Milestone: 4.8.12
Component: mc-core Version: master
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

Open a popup dialog (F2 user menu, Alt+` screen list, Alt+E charset) and then resize the window. Notice that the popup dialog moves upwards by 2 rows.

Attachments

mc-3173-popup-location.patch (1.5 KB) - added by egmont 3 years ago.

Change History

comment:1 follow-up: ↓ 2 Changed 3 years ago by egmont

Also note that these popups are really not centered horizontally.

In lib/widget/dialog.c: dlg_set_size(), if DLG_TRYUP is set, the window is moved up by two lines. If DLG_CENTER is unset at the same time, it's not specified compared to what it's moved upwards. And since dlg_set_size() is called over again on each resize, the window keeps moving up.

It's unclear to me what DLG_TRYUP without DLG_CENTER should mean, whether it should be declared as invalid (how?) or if dlg_set_size() should be tolerant here.

The logic of "if (y > 3) y -= 2" is weird, it's not monothonic: 4 gets decremented to 2, but 3 stays 3.

The comment of dlg_set_size():
"this function sets only size, leaving positioning to automatic methods"
is obviously incorrect.

Patch attached. It's not complete yet, the popup doesn't change the number of lines shown (e.g. when decrement the height so that the entries no longer fit, they move out of the terminal area instead of the widget decrementing the number of visible lines and adding a scrollbar), but it is better than the current version.

Changed 3 years ago by egmont

comment:2 in reply to: ↑ 1 Changed 3 years ago by andrew_b

  • Status changed from new to accepted
  • Owner set to andrew_b
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.12

Branch: 3173_popup_location
Initial changeset:957a3e36c0a0bd0f2dfba0ded0862af361175adb

Replying to egmont:

It's unclear to me what DLG_TRYUP without DLG_CENTER should mean, whether it should be declared as invalid (how?) or if dlg_set_size() should be tolerant here.

And do we really need the DLG_TRYUP flag?

Patch attached.

Thanks! Applied as 2nd commit in the branch. The 1st commit fixes the position of popup window itself.

comment:3 follow-up: ↓ 4 Changed 3 years ago by egmont

Doesn't compile due to obvious syntax error:
if (y > 3))

comment:4 in reply to: ↑ 3 Changed 3 years ago by andrew_b

Oops... Sorry. Fixed.

comment:5 Changed 3 years ago by slavazanko

  • Votes for changeset set to slavazanko
  • Branch state changed from on review to approved

comment:6 Changed 3 years ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from slavazanko to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

Merged to master: [5ae9521760dfd2dd31381d7d51f07f19d0662231].

git log --pretty=oneline 8ee4732..5ae9521

comment:7 Changed 3 years ago by andrew_b

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.