Add initialSearchTerm prop
#32
Conversation
|
@MrBartusek can you check that? |
|
@manoelneto Hey, thanks for the PR. What is the motivation for this change, and how would it help our users? What are the use cases for this property? |
|
the idea is to make the search term controlled, so we can pass initial term for example. |
What are the use cases of this? Can you give examples of how would it benefit the developer using this picker? |
|
Hey @MrBartusek, I'm not entirely sure if my solution is perfect, but I think it makes sense for the parent component to control the query term if needed. So, I came up with this approach that doesn't require too much code change. By the way, I noticed that in the Storybook, the way you're passing the initial query is a bit off—it seems like you're simulating DOM events to change the term, which feels a bit against React patterns. |
I want to know how would it be usefull to a developer. In which situation would anyone want to control the internal search term here
These are internal test files, they are ment to simulate user actions. |
|
It's very usefull. When we render the gif component, we pass the initial query term. As in this example, sending a shoutout in our platform.
We also have other use cases (celebrating birthdays) that is also usefull. |
|
Unrelated, not sure if you noticed, but I also added the powered by tenor image below the searchbar to our internal fork, which is also necessary to complain with tenor terms. |
For that reffer to: https://github.com/MrBartusek/gif-picker-react#what-to-know-before-using |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@MrBartusek I agree that this is a useful addition. Currently the only reason I have not implemented this library is that I have no way to provide this context when the library opens. So I get a list of unrelated gifs by default, instead of a list of gifs related to the chat message the user had already started typing. Please allow this functionality to exist or for a developer to at least have a way to override the default text so we can accomplish it. Thank you. |
|
@MrBartusek I can share an use case from our product, we have creators that upload to Tenor their own gallery of gifs, and all of their gifs have a common tag that is related to our product's name, so the initial search for the users is using this same common tag so they can easily pick without having to search... but they can still search if they want, just a matter of UX. We're migrating from Giphy to Tenor, let me know if there is a better approach if you don't agree to the initial search query. |
|
@MrBartusek I had a similar use case above. And as Giphy has now introduce a pricing plan, I suspect lots of people will be migrating to Tenor, with similar requests. For what it's worth, if the sole reason for not implementing this is concern over where you can post attribution for Tenor, just append a powered by Tenor statement right under the search input, as in @manoelneto screenshot above, or rely on the user to figure out where to post attribution and stay compliant. It will still look acceptable. |
|
Yeah, maybe a small label with "Tenor Results" under the search bar could work. Or, as @jaxomlotus said, just require the users to manage attribution. |
|
I didn't find in their terms, so suggesting here (lmk if it's agains't), but you can still show "Search Tenor" and allow initial search query to render/filter the relevant initial gifs, but once the user start typing that's what takes precedence. Initial search query is not the same as initial input value IMHO, for our product we only need to filter initial results and not control the input value itself. |
|
@jaxomlotus @TheNoobiCat For the attribution concerns - according to the atribution guide I wouldn't worry too much about it
We do have a placeholder and the fact that we put some default text in the search box on render doesn't change that. |
|
@MrBartusek I sent a different PR based on my original suggestion in case you agree/prefer the approach #33 |
src/stories/GifPicker.stories.tsx
Outdated
| export const InitialSearchTerm = { | ||
| ...Home, | ||
| args: { | ||
| ...Home.args, | ||
| initialSearchTerm: 'love' | ||
| } | ||
| }; |
There was a problem hiding this comment.
We already have Search story and that's just a duplicate. Please change Search story to use initialSearchTerm instead of user event rather than create new one
There was a problem hiding this comment.
all stories already have the initial search term, I'll just remove this.
MrBartusek
changed the title
initialSearchTerm prop
|
@manoelneto you have skipped some of my comments |
|
@manoelneto please also change |
done. sorry about that. |
it's already done |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Thank you! |
|
I've just published |
|
It works great! Thank you.
…On Sat, Sep 21, 2024 at 6:39 AM Bartosz Dokurno ***@***.***> wrote:
I've just published v1.4.0 please let me know if it works!
—
Reply to this email directly, view it on GitHub
<#32 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADWSSWJJLKNMIJ7CIGN3EDZXVEGNAVCNFSM6AAAAABMCRHCYGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRVGEZTQOJWHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This allows to control the search term by overriding the pickerContextValue.