Add right click popup for keyboard buttons#10220
Add right click popup for keyboard buttons#10220jrcleeman wants to merge 5 commits intoihhub:masterfrom
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Hi @jrcleeman , please fix code style issues and also clang-tidy problems. |
|
Hi @jrcleeman , I do not think that the proposed solution is flexible and actually it relies on special conditions. I suggest to add a separate field in |
ihhub
added
improvement
ihhub
changed the title
ihhub
left a comment
ihhub
left a comment
There was a problem hiding this comment.
Hi @jrcleeman , this version is way better than the original one. We are definitely on the right track. I left a few more comments here. Would you mind to take a look at them?
| // This is used only for buttons which should have pressed state for some layouts. | ||
| bool isInvertedRenderingLogic{ false }; | ||
|
|
||
| std::string helpTitle; // e.g., "Space", "Backspace", "Shift" |
There was a problem hiding this comment.
Please put newly added comments before member declaration as it's done above.
|
|
||
| const size_t letterRows = buttonLetters.size(); | ||
|
|
||
| for ( size_t row = 0; row < buttons.size(); ++row ) { |
There was a problem hiding this comment.
Please use ranged loops here to avoid indexing arrays.
| fheroes2::showStandardTextMessage( title, _( "Insert this character." ), Dialog::ZERO ); | ||
| } | ||
| else { | ||
| // Last resort. |
There was a problem hiding this comment.
It feels off. We must handle everything. Such scenario must be asserted without any assumptions.
| renderer.setCursorPosition( le.getMouseLeftButtonPressedPos() ); | ||
| } | ||
|
|
||
| // right click help for any key on the virtual keyboard. |
There was a problem hiding this comment.
Please be consistent with comments: every comment starts from a capital letter and also always on a new line. Also, it usually ends with a full stop to give a meaning of a complete explanation.
|
|
||
| KeyboardButton( std::string input, const fheroes2::Key kbKey, const fheroes2::Size & buttonSize, const bool isEvilInterface, | ||
| std::function<DialogAction( KeyboardRenderer & )> actionEvent ) | ||
| std::function<DialogAction( KeyboardRenderer & )> actionEvent, std::string helpT = {}, std::string helpD = {} ) |
There was a problem hiding this comment.
Please do not use default values for input arguments. It's usually a risky practice. I also suggest to move these arguments just after input where they logically belong to. In the code we can use {} to indicate empty strings.
| return true; | ||
| } | ||
|
|
||
| // Generic character help (for normal character keys) |
There was a problem hiding this comment.
Here we just need to display "Insert this character" with the character, and then assert that both title and description are empty.
|
Closing this pull request in favor of #10585 since the author of this PR vanished. |
2 participants
Fixes #10152