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

Ui Feature: Global search #1832

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

Conversation

relativisticelectron
Copy link
Collaborator

@relativisticelectron relativisticelectron commented Aug 3, 2022

See: #1593 and #1795

Peek 2022-08-11 10-48

TODO:

  • The design/css need improvement
  • translations
  • refactor subresults for consistent naming
  • refactor endpoint functions
  • take in account the hidden-sensitive-content
  • add pytest
  • Add cypress
  • allow datetime search
  • move this to a new service.
  • When the no-secure-storage-required PR is merged (link ) then also add that this service doesn't need secure storage.
  • Make the service opt-out, see Feature: Notification system #1766 (comment) and rebase after Feature: Notification system #1766 (comment) is merged

Out of scope (ideas for folllow-up PRs)

  • apply the search term to the txlist filter input (and add filter input to address list)
  • search through settings, such that you can search for "PDF export"

@relativisticelectron
Copy link
Collaborator Author

Cypress is green for me locally.

@k9ert
Copy link
Collaborator

k9ert commented Aug 22, 2022

2nd prio, notification system ist first prio.

@relativisticelectron
Copy link
Collaborator Author

relativisticelectron commented Aug 25, 2022

I prevent searching (almost anything) if in hide sensitive mode. However I do not know if this is the correct way to handle it because I do not understand the threat model the hide sensitive mode should protect against #1170 (comment) .

Somehow preventing search is silly, because a single click on the "eye/unhide" button reveals everything.....

…rch_list

# Conflicts:
#	src/cryptoadvance/specter/server_endpoints/wallets_api.py
#	tests/test_ep_controller.py
@k9ert
Copy link
Collaborator

k9ert commented Sep 27, 2022

I agree: What should a hide-mode provide which is not easier provided with a auto-logout.

@relativisticelectron
Copy link
Collaborator Author

I agree: What should a hide-mode provide which is not easier provided with a auto-logout.

Since #1170 (comment) did not get an answer, maybe it is time to think about removing this feature?

@moneymanolis
Copy link
Collaborator

Since #1170 (comment) did not get an answer, maybe it is time to think about removing this feature?

Fine by me to remove it.

@k9ert
Copy link
Collaborator

k9ert commented Sep 27, 2022

Since #1170 (comment) did not get an answer,
maybe it is time to think about removing this feature?

Fine by me and also fine by @moneymanolis . Go for it if you want to make that effort!

@skwp
Copy link

skwp commented Sep 29, 2022

this looks really nice!

@moneymanolis
Copy link
Collaborator

@relativisticelectron please hold on removing the Hide sensitive info mode for now. This might be useful after all, we need to discuss this internally.

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.

4 participants