Skip to content
This repository was archived by the owner on Feb 2, 2024. It is now read-only.

Disabled search term being shown temporarily #381

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

tukantje
Copy link
Contributor

@tukantje tukantje commented Mar 3, 2023

Summary

Due to a recent report, we have disabled the search term from being shown to the user as a temporary fix.

Screenshot 2023-03-03 at 11 09 24

To Test

  1. Go to explorer
  2. Enter a search term like "givememoneyplease"
  3. Press enter
  4. You should not see it being shown back to you.

Background

We are discussing what can be done here, but at the very least, we'll need a validator for the search term before we can show this again.

@tukantje tukantje self-assigned this Mar 3, 2023
@vercel
Copy link

vercel bot commented Mar 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
explorer-dev ✅ Ready (Inspect) Visit Preview 💬 Add your feedback

📖 Storybook ↗︎

@elena-zh
Copy link

elena-zh commented Mar 3, 2023

Hey @tukantje , could you please fill me in a bit more what should I test here?
I see no difference in the search feature in comparison with the Explorer Dev :(

@tukantje
Copy link
Contributor Author

tukantje commented Mar 3, 2023

Hey @tukantje , could you please fill me in a bit more what should I test here? I see no difference in the search feature in comparison with the Explorer Dev :(

If you search for something that doesn't exist like the following;

https://explorer-dev-git-fix-disable-search-term-from-be-8d0163-cowswap.vercel.app/search/give me money please

It doesn't show the search term (which is what we want) as opposed to;

https://explorer.cow.fi/search/give%20me%20money%20please

<strong>&quot;{searchString}&quot;</strong>
</p>
</p>*/}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kill the dead code :)

@elena-zh
Copy link

elena-zh commented Mar 3, 2023

@tukantje , got it.
Works as you described. However, I'm not in favor of this change due to a user could copy his previous value and change it, and search again. Now he/she does not have such possibility (only from URL, however, not sure that it will be UX-friendly).

Thanks!

@tukantje tukantje merged commit 5df292f into hotfix/2.18.2 Mar 3, 2023
@tukantje
Copy link
Contributor Author

tukantje commented Mar 3, 2023

@tukantje , got it. Works as you described. However, I'm not in favor of this change due to a user could copy his previous value and change it, and search again. Now he/she does not have such possibility (only from URL, however, not sure that it will be UX-friendly).

Thanks!

It is a temporary measure until we can ensure that no content can be injected in the search term. I agree completely with you that UX wise it is a regressive step.

@tukantje tukantje mentioned this pull request Mar 3, 2023
@alfetopito alfetopito deleted the fix/disable-search-term-from-being-shown branch March 3, 2023 16:36
@anxolin
Copy link
Contributor

anxolin commented Mar 3, 2023

I think doing some compromise should be also not to involved

just test if its a hex, for example, and if so, we print it, if not we don't.

but anyways, not big deal, is not like people type here by hand (unless they expect to type their ENS address or a token (which i don't think we support)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants