GTNewHorizons / NotEnoughItems

GNU Lesser General Public License v3.0
50 stars 67 forks source link

Added ability to have the NEI search be auto-focused on open. #492

Closed OvermindDL1 closed 1 month ago

OvermindDL1 commented 1 month ago

A feature that a few people I knew, including myself, were wanting.

Added an option within the Settings->Inventory called Focus Search Widget on Open Below the Center Search Widget option as they are closely related options, default Off to keep existing functionality as-is.

When that option is enabled then when the NEI UI is shown on a GUI then the item search field will be automatically focused.

However, if the the mouse is moved/clicked/anything before any further keyboard interaction takes place then the search box will be automatically blurred (focus removed). This allows something like immediately moving the mouse to an item and hitting r to see the recipe or so continue to work unimpeded.

If instead a keyboard key is pressed first then the box will stay focused as normal, even if the mouse is later moved.

I had 3 different people test it and they didn't find any edge cases as well as I didn't find any myself, but is someone here able to test this first just to verify with more eyes (I have a couple more people scheduled to test tomorrow as well).

Again, the option is default disabled and if not enabled then existing NEI functionality should remain identical.

I created a language entry and an appropriate simple comment should accompany the configuration file setting as well for those that view it directly.

Yoghurt4C commented 1 month ago

fmita

slprime commented 1 month ago

What problem needs to be solved with this PR?

I found 2 problems: 1.The terminals have autofocus, but this PR breaks it (when this function on)

  1. When returning to a previous recipe in GUIRecipe, you need to move or click the mouse before each backspace. It is very uncomfortable
Dream-Master commented 1 month ago

@OvermindDL1

OvermindDL1 commented 1 month ago
  1. What terminals are these?

  2. I'll look at the GUIRecipe thing, probably some simple way to detect it, just need to find the right link to figure that out.

A related question, if something else has acquired focus I'm guessing there must be some way to determine if and what that is, does anyone have this information that could save me time? If something has already acquired focus first then it would make sense not to override it.

I'll look at this at my next opportunity, I'm hoping this coming afternoon but it could potentially be up to a few days.

Thanks much for the feedback!

slprime commented 1 month ago
  1. What terminals are these?
  2. I'll look at the GUIRecipe thing, probably some simple way to detect it, just need to find the right link to figure that out.

A related question, if something else has acquired focus I'm guessing there must be some way to determine if and what that is, does anyone have this information that could save me time? If something has already acquired focus first then it would make sense not to override it.

I'll look at this at my next opportunity, I'm hoping this coming afternoon but it could potentially be up to a few days.

Thanks much for the feedback!

Crafting Terminal from ME, for example

OvermindDL1 commented 1 month ago

Crafting Terminal from ME, for example

So had a few minutes at work aaaand what actual hell is MC's gui system, it really has everything check inputs on their own so every subclass of a gui window is forced to do its own invented focus stuff so there's no unified somewhat normal api for this stuff...

Yeah I'm making a denylist then. All of AE's group is going to be on it, guessing I should add any addons that add terminals with their own search on them as well, anyone have a list of such AE addons groups so I can just add them entirely to the list as well? Anything else perhaps too?

Hmm, maybe I should just have an allowlist instead of a denylist... that seems easier and safer... I'll do that when I get another few minutes at work to look at it more.

OvermindDL1 commented 1 month ago

Going to be doing a few smaller commits instead of one large one to break up what I'm doing, I'll respond again when done...

slprime commented 1 month ago

Configuration in config is not the best way to set inventory list because it may be large

Please use file. for example serialHandlersFile, heightHackHandlersFile, or handlerOrderingFile

slprime commented 1 month ago

and please use spotlessApply command before commit

OvermindDL1 commented 1 month ago

Configuration in config is not the best way to set inventory list because it may be large

I'm not storing any inventory in the config, that new mapping is only for classname's where the mods don't already implement an appropriate interface (incoming commit once I get a bit more time at work, this was requested by @eigenraven and will/should keep the list quite small to begin with, probably default empty actually, as they intend to add it to various mods here), and so should be quite short.

Shall I still have them be placed in such a file? Simple enough if so, though it doesn't seem like that pattern of those files within NEI supports any live reloading like config adjustments do however, if that's fine?

and please use spotlessApply command before commit

I do repeatedly, though wish there were a git precommit hook that did it automatically, might just make my own, though the project (or RFG's gtnh setup perhaps for all) should probably set up its own.

Dream-Master commented 1 month ago

@OvermindDL1 if you add more stuff in the future and are part of the development team, Spotless will be added automatically

slprime commented 1 month ago

Shall I still have them be placed in such a file? Simple enough if so, though it doesn't seem like that pattern of those files within NEI supports any live reloading like config adjustments do however, if that's fine?

  1. yes, please use file.
  2. live reloading not needed for this
OvermindDL1 commented 1 month ago

@OvermindDL1 if you add more stuff in the future and are part of the development team, Spotless will be added automatically.

I just told eigenraven how precommit hooks can be setup per-system so now so maybe we'll see an RFG task that can do it in an update (I'm hoping? It would be an explicit call, not automatic just so it's not a "surprising" change).

  1. yes, please use file.

Done and committed.

All prior requests should be done (this also fixed the backspace issue as that recipe display screen was just not put on the allowlist). Is anything else wanted that I can add while I still have some time today? Perhaps some mods screens that would have a definite benefit for having autofocus support that can be added to the allowlist (that won't just implement the interface that is)?

OvermindDL1 commented 1 month ago

And yes, I ran spotlessApply right before committing, and locally running ./gradlew spotlessCheck returns success.

OvermindDL1 commented 1 month ago

Spotless is crashing in the github runner (though it's successful locally)?

slprime commented 1 month ago

please dont use .* in import

OvermindDL1 commented 1 month ago

please dont use .* in import

I didn't, that was intellij converting it. I'm not seeing a style definition in the project for it to use so it used defaults (perhaps something else for RFG to generate?). One moment...

OvermindDL1 commented 1 month ago

Well that was annoying, I'd fix it and intellij would undo it on save, don't feel like looking for the config so to nvim, done.

OvermindDL1 commented 1 month ago

If anyone is curious or asks or so, a .editorconfig file containing (in the root project directory or a parent directory thereof, you can apparently control all of intellijs style configurations via editorconfig nowadays):

[*.java]
ij_java_class_count_to_use_import_on_demand = 999
ij_java_names_count_to_use_import_on_demand = 999

when intellij has the official EditorConfig plugin installed (default, at least in ultimate) will do it without any jetbrain xml pollution in the project nor needing to configure each project individually or pollute global settings.

OvermindDL1 commented 1 month ago

(As an aside, tempted to add a similar feature to mouse too, like if one moves the mouse after typing in the search and then hits 'r' or whatever the button is configured to then it would show the recipe instead of continuing to type in the search... configurable as well of course...)