-
Notifications
You must be signed in to change notification settings - Fork 241
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
UI Feature: Global search across tx, addresses, amounts, xpubs #1795
UI Feature: Global search across tx, addresses, amounts, xpubs #1795
Conversation
and added searches of more addresses
❌ Deploy Preview for specter-desktop-docs failed.
|
WOW, this looks awesome! Will have a look soon! |
logger = logging.getLogger(__name__) | ||
|
||
|
||
class HtmlElement: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds cool but i don't have an idea from that description. Maybe it's a good candidate for some tests? It looks like it's quite easy to write some tests for it and that would give a much better picture how this works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linking (via child/parent) multiple HtmlElements rebuilds the logical structure of the specter UI. It also attaches the search function, and other actions (such as pasting the search string in tx-table
).
Yes, in the end this needs tests. However I am not sure (yet) the "highlighting" of the buttons is the best UI. Other UI approaches could be taken, e.g.:
- Dropdown result list (similar to a webbrowser search bar). A click on the result lets you jump to the correct endpoint (with filtered tx-table)
I wanted to get started and see how this approach feels like in the UI. Do you have an elegant way of preserving the search_term of the global_search_input across when a user loads a different endpoint?
Do you have preferences what would look better? The highlighting, or the "webbrowser" dropbdown search results?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserving the search-term should be easy via storing it in the session?! I try to get out of the way of UI judgement. Don't have a good track record there. @moneymanolis @b30wulffz what do you think?
@result.setter | ||
def result(self, result): | ||
if self.children: | ||
raise "Setting results is only allowed for end nodes" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raising a string? Shouldn't you raise an Exception or a descendant from an exception?
|
||
def do_global_search(search_term, specter): | ||
"Builds the HTML Tree if ncessary (only do it once) and then calls the functions in it to search for the search_term" | ||
global HTML_ROOT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether global
is a good idea?! Is that possible with multi-user? How expensive is the build-up of the tree? Maybe we can do it every time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right, I didn't think about multi-user yet. Please do not put in too much effort now to review the code; I didn't put any emphasis on good structure yet. Before I put more work in the code, I am seeking feedback, if this feature is wanted, and how the UI should look like (which then probably leads to lots of code changes).
wallets = HtmlElement(html_root, id="toggle_wallets_list") | ||
devices = HtmlElement(html_root, id="toggle_devices_list") | ||
|
||
def search_in_structure(search_term, l): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Puh, this function in function-scope is imho quite horrible. Especially evil is the example below where you're not even returning a value but just changing variables which are in the scope. To me, this is a very confusing programming-model.
Shouldn't it be possible to include these methods in the HtmlElement-class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, this needs a restructuring.
@moritzwietersheim Maybe you can also give feedback here. How do you like the current way of visualizing the results? An alternative might be:
@GBKS i think you might also be interested in this interesting way of showing results! |
@relativisticelectron I'd need to first understand the goals associated with this PR before we can talk UI options etc. What do you want to search for? When is the global search used / sensible and when the search bar of the tx table? So far, I could only gather: "feature to quickly search all wallets for an address." - which is fine if you want to double check whether an address belongs to one of your wallets and you don't remember which one. My rough first intuition is, that we talk about a two step approach here. (1) The global search tells you in which wallet(s) to look for, (2) and then, say, the search in the tx table takes over |
The original inspiration is gmail. The search searches everything, is fast and changes the experience/behavior of "I have to remember in which folder the email is" to "just search for it". UX: Search (and see results immediately) --> Click on result --> See result fully. --> Go back to result list if it was the wrong one In Specter I had the experience: In which wallet was the address and was it used already? I would like to mirror the gmail UX flow with as few steps as possible (I think the current PR doesn't do that, because there are too many clicks, but it was a first attempt). Would you also like to see such a search in Specter? Then I could rewrite the PR. (Not sure from where to get such a nice "browser search bar" like web component though. Any suggestions for a "browser search bar" webcomponent?) |
OK, I understand it better now, thanks. Another question, why don't we just improve the UX flow in the wallets overview. If I had to check whether an address belongs to any of my wallets, I'd go there and type the address in the search bar: We could add highlighting of the wallet in the sidebar here to clarify which wallet the address is belonging to. |
I have to redesign this search. Currently getting to the result needs way too many clicks.... Closing this PR for now. If anyone knows a good webcomponent for a rich-text dropdown, then please write it here. |
See: #1593
For me most important is the feature to quickly search all wallets for an address.
TODO:
If you have feedback, e.g., how to make the UI nicer, let me know.