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

[MainMenu] Add clear button and icon for search one. #10363

Merged

Conversation

Andrey2470T
Copy link
Contributor

This PR allows to purge the user's search results immediately like as in the creative inventory by just pressing the new 'Clear' button. Also, this adds an icon for the current search button, instead the text. It modifies 'Online' tab and the package manager.

'Online' tab:
Screenshot_20200902_235604

Package Manager:
Screenshot_20200902_235639

To do

This PR is Ready for Review.

@paramat
Copy link
Contributor

paramat commented Sep 3, 2020

I assume the 'clear' and 'search' icons are taken from MTG creative mod?
If so, those textures need adding to the texture credits here

minetest/LICENSE.txt

Lines 22 to 25 in 4ba5046

paramat:
textures/base/pack/menu_header.png
textures/base/pack/next_icon.png
textures/base/pack/prev_icon.png

EDIT: Done.

@Andrey2470T
Copy link
Contributor Author

May anybody please review quickly prior to merging? It doesn't take much time surely as this PR is
pretty minor.

@numberZero
Copy link
Contributor

I’d prefer search-erase-refresh order, more like the creative inventory. Tooltips would be nice too.

@Andrey2470T
Copy link
Contributor Author

Andrey2470T commented Sep 14, 2020

I think the clear button yet should be first as this is common for the most of graphic interfaces. I'm unsure about tooltips, I think the icons on the buttons are already intuitively understandable what their buttons implement.

builtin/mainmenu/dlg_contentstore.lua Outdated Show resolved Hide resolved
builtin/mainmenu/dlg_contentstore.lua Outdated Show resolved Hide resolved
@Andrey2470T
Copy link
Contributor Author

The PR is ready and already reviewed by one core dev. So, merge?

@Andrey2470T
Copy link
Contributor Author

Andrey2470T commented Nov 23, 2020

What is a current status of my PR? It is very easy and has been hanging out in an undefinitive state already two months. No feedback from the core devs up-to-day. Is it approved? If yes, may it be merged now?

@sfan5
Copy link
Member

sfan5 commented Dec 7, 2020

The broken indentation could be seen in the lua_lint run too: https://github.com/minetest/minetest/pull/10363/checks?check_run_id=1474999370#step:5:71
You didn't have to wait for me to tell you. Generally a failing build needs to be fixed before the PR can be merged at all.

@Andrey2470T
Copy link
Contributor Author

@sfan5: I`ve fixed all what was necessary: added actual tabs in my editor and removed the whitespaces. May it be merged now?

@Andrey2470T
Copy link
Contributor Author

Search-clear-refresh order how was requested by sfan5 on the irc:
Screenshot_20201210_212407
Screenshot_20201210_212419

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

Tested.

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Works

@Andrey2470T
Copy link
Contributor Author

Thanks. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants