-
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
Changes from all commits
660faa7
89ddef2
44d4571
9c64aa1
19dd23f
27245c7
5b300c6
13030a3
f1fcc61
e49523f
fd06496
6699990
263b3fd
487c249
d5705f2
56bd036
c4ac959
f051ab0
4cdc69e
176f81b
893091f
6458fcc
3367005
391a5cf
52b121b
5a72501
387f8e1
2017ffc
663e47a
4efe607
c903da4
f1c763a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,260 @@ | ||
import os | ||
import logging | ||
|
||
from flask import url_for | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
class HtmlElement: | ||
"This is a way to reconstruct the HTML Logical UI Tree and sum up results nicely" | ||
|
||
def __init__( | ||
self, | ||
parent, | ||
id=None, | ||
function=None, | ||
children=None, | ||
visible_on_endpoints="/", | ||
filter_via_input_ids=None, | ||
): | ||
self.parent = parent | ||
if self.parent: | ||
self.parent.children.add(self) | ||
self.children = children if children else set() | ||
self.id = id if id else set() | ||
self._result = None | ||
self.function = function | ||
self.visible_on_endpoints = visible_on_endpoints | ||
self.filter_via_input_ids = filter_via_input_ids | ||
|
||
@property | ||
def result(self): | ||
if self._result: | ||
return self._result | ||
else: | ||
return sum([child.result for child in self.children]) | ||
|
||
@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 commentThe 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? |
||
self._result = result | ||
|
||
def calculate_end_nodes(self): | ||
if not self.children: | ||
return [self] | ||
|
||
end_nodes = [] | ||
for child in self.children: | ||
end_nodes += child.calculate_end_nodes() | ||
return end_nodes | ||
|
||
def reset_results(self): | ||
end_nodes = self.calculate_end_nodes() | ||
for node in end_nodes: | ||
node.result = None | ||
|
||
def flattened_sub_tree_as_json(self): | ||
result_list = [self.json()] | ||
for child in self.children: | ||
result_list += child.flattened_sub_tree_as_json() | ||
return result_list | ||
|
||
def json(self): | ||
d = {} | ||
d["id"] = self.id | ||
d["children"] = self.children | ||
d["result"] = self.result | ||
d["visible_on_endpoints"] = self.visible_on_endpoints | ||
d["filter_via_input_ids"] = self.filter_via_input_ids | ||
return d | ||
|
||
|
||
HTML_ROOT = None | ||
|
||
|
||
def build_html_elements(specter): | ||
""" | ||
This builds all HtmlElements that should be highlighted during a search. | ||
It also encodes which functions will be used for searching. | ||
|
||
Returns: | ||
HtmlElement: This is the html_root, which has all children linked inside | ||
""" | ||
html_root = HtmlElement(None) | ||
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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I agree, this needs a restructuring. |
||
count = 0 | ||
for item in l: | ||
if isinstance(item, dict): | ||
count += search_in_structure(search_term, item.values()) | ||
elif isinstance(item, list): | ||
count += search_in_structure(search_term, item) | ||
elif search_term.lower() in str(item).lower(): | ||
count += 1 | ||
return count | ||
|
||
def add_all_in_wallet(wallet): | ||
sidebar_wallet = HtmlElement(wallets, id=f"{wallet.alias}-sidebar-list-item") | ||
wallet_names = HtmlElement( | ||
sidebar_wallet, | ||
id="title", | ||
function=lambda x: search_in_structure(x, [wallet.alias]), | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
transactions = HtmlElement( | ||
sidebar_wallet, | ||
id="btn_transactions", | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
filter_via_input_ids=( | ||
f"tx-table-{wallet.alias}", | ||
"shadowRoot", | ||
"search_input", | ||
), | ||
) | ||
transactions_history = HtmlElement( | ||
transactions, | ||
id=( | ||
f"tx-table-{wallet.alias}", | ||
"shadowRoot", | ||
"btn_history", | ||
), | ||
function=lambda x: search_in_structure(x, wallet.txlist()), | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
transactions_utxo = HtmlElement( | ||
transactions, | ||
id=( | ||
f"tx-table-{wallet.alias}", | ||
"shadowRoot", | ||
"btn_utxo", | ||
), | ||
function=lambda x: search_in_structure(x, wallet.full_utxo), | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
|
||
addresses = HtmlElement( | ||
sidebar_wallet, | ||
id="btn_addresses", | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
addresses_recieve = HtmlElement( | ||
addresses, | ||
id=( | ||
f"addresses-table-{wallet.alias}", | ||
"shadowRoot", | ||
"receive-addresses-view-btn", | ||
), | ||
function=lambda x: search_in_structure( | ||
x, wallet.addresses_info(is_change=False) | ||
), | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
addresses_change = HtmlElement( | ||
addresses, | ||
id=( | ||
f"addresses-table-{wallet.alias}", | ||
"shadowRoot", | ||
"change-addresses-view-btn", | ||
), | ||
function=lambda x: search_in_structure( | ||
x, wallet.addresses_info(is_change=True) | ||
), | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
|
||
recieve = HtmlElement( | ||
sidebar_wallet, | ||
id="btn_receive", | ||
function=lambda x: search_in_structure(x, [wallet.address]), | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
|
||
send = HtmlElement( | ||
sidebar_wallet, | ||
id="btn_send", | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
unsigned = HtmlElement( | ||
send, | ||
id="btn_send_pending", | ||
function=lambda x: search_in_structure( | ||
x, [psbt.to_dict() for psbt in wallet.pending_psbts.values()] | ||
), | ||
visible_on_endpoints=[ | ||
url_for("wallets_endpoint.wallet", wallet_alias=wallet.alias) | ||
], | ||
) | ||
|
||
def add_all_in_devices(device): | ||
sidebar_device = HtmlElement(devices, id=f"device_list_item_{device.alias}") | ||
device_names = HtmlElement( | ||
sidebar_device, | ||
id="title", | ||
function=lambda x: search_in_structure(x, [device.alias]), | ||
visible_on_endpoints=[ | ||
url_for("devices_endpoint.device", device_alias=device.alias) | ||
], | ||
) | ||
device_keys = HtmlElement( | ||
sidebar_device, | ||
id="keys-table-header-key", | ||
function=lambda x: search_in_structure(x, [key for key in device.keys]), | ||
visible_on_endpoints=[ | ||
url_for("devices_endpoint.device", device_alias=device.alias) | ||
], | ||
) | ||
|
||
for wallet in specter.wallet_manager.wallets.values(): | ||
add_all_in_wallet(wallet) | ||
for device in specter.device_manager.devices.values(): | ||
add_all_in_devices(device) | ||
return html_root | ||
|
||
|
||
def apply_search_on_dict(search_term, html_root): | ||
"Given an html_root it will call the child.function for all childs that do not have any children" | ||
end_nodes = html_root.calculate_end_nodes() | ||
for end_node in end_nodes: | ||
end_node.result = end_node.function(search_term) | ||
return html_root | ||
|
||
|
||
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 commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure whether There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). |
||
if not HTML_ROOT: | ||
HTML_ROOT = build_html_elements(specter) | ||
else: | ||
HTML_ROOT.reset_results() | ||
print(HTML_ROOT) | ||
|
||
if search_term: | ||
apply_search_on_dict(search_term, HTML_ROOT) | ||
print(HTML_ROOT.flattened_sub_tree_as_json()) | ||
return { | ||
"tree": HTML_ROOT, | ||
"list": HTML_ROOT.flattened_sub_tree_as_json(), | ||
"search_term": search_term, | ||
} |
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.:
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?