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

Implements issue #5, Ability to add items to layout/tag by name (search). #45

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Wynadorn
Copy link
Contributor

I've implemented the ability to add an item to the layout via search. #5
https://www.youtube.com/watch?v=E01dj89zxzg

Have a look if you think this implementation is sufficient.
Could use a code review as it's my first contribution to your project, but it seems to be working great!

Definitely something I was looking for on my group iron.

When adding a bunch of items a keybind for this action would be nice too.

Maybe it'd be nice to add the item on the first visible index, when adding items to a long tab you have to scroll up and drag it over to where you where trying to add something

@geheur
Copy link
Owner

geheur commented Nov 11, 2021

I think having the add item to layout option on right-clicks in the bank over empty spaces might be better, what do you think?

  • Item is pre-positioned, no need to drag it to the spot you want it at.

downsides:

  • it's not going to be obvious to people because they don't right-click empty space very often.

A checkbox for adding an item's variants might be neat, for stuff like barrows armor.

Also, looking at the code (haven't run it yet), it looks like adding an item to a non-open tab will NPE.

@Wynadorn
Copy link
Contributor Author

Wynadorn commented Nov 11, 2021

add item to layout option on right-clicks in the bank over empty spaces

Yep, that'd be better

A checkbox for adding an item's variants

Maybe, I wouldn't use it personally, but I can see how people would like separate placeholders per variant

adding an item to a non-open tab will NPE

Oops! Yeah I wouldn't be surprised if it did, first point should take care of it

I've also gone and implemented the ability to open the add item search dialog via hotkey which is really really nice when creating a bunch of them. It's customizable in the settings tab. The default is Ctrl+A (for add). Ctrl+N could be an alternative for the default binding.
I can push that later.

@geheur
Copy link
Owner

geheur commented Nov 11, 2021

separate placeholders per variant

The variants thing I mentioned is not for separate placeholders but for adding variants of an item to the tag only, not the layout, so that if you add barrows armor and it degrades it will still show up in the tag. (EDIT: this is only useful for a few items so it's probably not worth doing anyways).

ctrl-A

I don't like the idea of a default keybind because people will have to go to the config panel to even know that it exists, and if they do that they may as well set it themselves. This avoids people asking "why am I no longer able to press ctrl-a" in runelite discord (assuming this can actually happen, it depends on how you implement the hotkey I guess).

first point should take care of it

Not if they close the tab for some reason.

lmao we both made the same errors with not putting empty lines after comments, lol.

@Wynadorn Wynadorn force-pushed the add-item branch 2 times, most recently from 0eebae4 to 1a139f9 Compare November 12, 2021 12:21
@Wynadorn Wynadorn closed this Nov 12, 2021
@Wynadorn Wynadorn reopened this Nov 12, 2021
@Wynadorn
Copy link
Contributor Author

Wynadorn force-pushed the add-item branch from 1a139f9 to efad26a 7 minutes ago

Cleaned this branch up a little

@Wynadorn Wynadorn changed the title Implements the ability to add items to layout/tag by name (search). Implements issue #5, Ability to add items to layout/tag by name (search). Nov 16, 2021
@geheur geheur force-pushed the master branch 3 times, most recently from 09dc68b to af0f042 Compare March 7, 2023 05:01
@geheur geheur force-pushed the master branch 6 times, most recently from f73df36 to d9f3ae7 Compare May 27, 2023 02:35
@geheur geheur force-pushed the master branch 2 times, most recently from 087dc5b to 0d16889 Compare March 16, 2024 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants