#8712 closed patch (fixed)
Possible patch for FR 1754616
| Reported by: | bluegr | Owned by: | bluegr |
|---|---|---|---|
| Priority: | normal | Component: | GUI |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
FR 1754616 asks to delete games using the delete key
I've implemented this in the attached patch, by modifying the list widget control, however it's not generic as the other commands, i.e. when "enter" is pressed, the current list item is activated, but, with this patch, when "delete" is pressed, the current game's configuration is removed (i.e it's assumed that only one listwidget control is used, the one that has all the game configurations).
AFAIK, list widgets are not used in any place other than the list of games, but if we do use a list widget someplace else, the delete key will be bound to "Remove game configuration" for that list, which is not a generalized command, but it's specific for the game list widget.
I'm not quite sure how this can become more generalized, so I'm submitting it as a patch for people who know more about ScummVM's GUI :)
Ticket imported from: #1755666. Ticket imported from: patches/817.
Attachments (1)
Change History (9)
by , 17 years ago
| Attachment: | delete game with del key.patch added |
|---|
comment:1 by , 17 years ago
There are actually few other places where ScummVM uses the list widget. Perhaps most noticeably, the SCUMM save/load dialog.
Anyway, the widgets should probably not have that kind of specific knowledge of what they're being used for. It would probably be better to use the "hotkey" handling (each button can have a hotkey assigned to it) somehow, but there are at least two problems with that approach: The hotkey is an ASCII code, not a "keycode", and the list widget steals the focus so we never get to the hotkey handler anyway.
comment:2 by , 17 years ago
Well, there are two approaches to solve this that I can think of: 1) create an class which inherits from listwidget (e.g. gameslistwidget), which handles the delete key 2) (a bit clumsy) add a check in the delete key case, which checks if the current list is "launcher_list" (the list of games)
I'm not sure about the hotkey class, as you said yourself that the list widget steals the focus. The Add/Edit/Remove game buttons have the "A", "E", "R" hotkeys assigned to them, but they're never triggered as the list itself has focus. So if we press "A", "E" or "R", the current item shifts to the one that starts with one of those letters (e.g. "Elvira" for "E")
comment:3 by , 17 years ago
| Owner: | set to |
|---|
comment:4 by , 17 years ago
The focus stealing is slightly less of a problem than I first thought. The list widget doesn't steal all keypresses. If the key isn't handled (e.g. the delete key, unless - I assume - in edit mode), the keypress passes through to the hotkey handling after all.
But the hotkey is still an ASCII code. Fingolfin might have opinions on whether or not that should be changed.
comment:5 by , 17 years ago
In the current form, the patch is not OK.
But if you simply change "kListRemoveGameCmd" to "kListItemRemovedCmd" or "kListItemRemovalRequestedCmd", it becomes generic enough, I'd say.
A subclass should be avoided. as for hotkeys: Those are only implemented for button widgets, and indeed are rather hackish right now - but I don't think we need to worry abot those here.
BTW, I' would catch both delete *and* backspace to trigger removal.
comment:6 by , 17 years ago
Thanks fingolfin, I've made the changes you requested, and the implementation is now generic :)
I've submitted this as commit #28131
Closing this
comment:7 by , 17 years ago
| Owner: | changed from to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:8 by , 6 years ago
| Component: | → GUI |
|---|
Delete a game's configuration with the del key