Allow to specify a different function to fetch results#82
Allow to specify a different function to fetch results#82theinterned merged 11 commits intogithub:mainfrom
Conversation
MaximeRnR
force-pushed
the
add-custom-way-to-fetch-results
branch
from
46920cc to
691e296
Compare
theinterned
left a comment
theinterned
left a comment
There was a problem hiding this comment.
Hello 👋 thank you for this contribution!
I left a few small notes, but I have a larger question about what it is you are wanting to achieve here.
Can you provide a bit more detail about why this feature should be added to the library? What specifically is your use case that the current implementation doesn't cover? Thanks again.
MaximeRnR
force-pushed
the
add-custom-way-to-fetch-results
branch
from
691e296 to
ba350f1
Compare
|
Hello @theinterned 👋 ! Thanks for you review! Currently, the way data are fetched is implemented in the component. With this new property, the developer can specify the way it wants the data to be fetched. For instance, my options are stored in a global variable, I can tell the component to retrieve it this way. Another example, I already have a end point that give a list of name as json, I can tell the component that the way to retrieve thoses names is to call this endpoint then map those data as Also the developer could want to add headers or auth token to its request. As long as the Hope I was clear thanks for your time ! ✌️ |
MaximeRnR
force-pushed
the
add-custom-way-to-fetch-results
branch
from
ba350f1 to
b396ba3
Compare
This commit adds fetchResult property to AutoCompleteElement; That property can only be set using script; If set, the property will be call in autocomplete.ts#fetchResult method;
MaximeRnR
force-pushed
the
add-custom-way-to-fetch-results
branch
from
b396ba3 to
2f791d2
Compare
theinterned
left a comment
theinterned
left a comment
There was a problem hiding this comment.
Okay thank you for the explanation!
I like the general direction. I've left some suggestions on how this could be simplified a bit.
MaximeRnR
force-pushed
the
add-custom-way-to-fetch-results
branch
from
c032a7c to
a264de9
Compare
Co-authored-by: Ned Schwartz <ned@theinterned.net>
Co-authored-by: Ned Schwartz <ned@theinterned.net>
Co-authored-by: Ned Schwartz <ned@theinterned.net>
Co-authored-by: Ned Schwartz <ned@theinterned.net>
MaximeRnR
force-pushed
the
add-custom-way-to-fetch-results
branch
from
a264de9 to
0cc8573
Compare
|
Thanks for the feedbacks ! I did the changes you suggested ! I like the simplicity of the test 💯 ! I overthinked it with the mock fetch 😄 |
|
I do not know the way you want the commits to be; I thought of squashing them to only 1 commit. |
theinterned
left a comment
theinterned
left a comment
There was a problem hiding this comment.
👏 thanks for all the updates! I left one additional suggestion then I think this is good to go.
As for squashing the commits: I wouldn't worry about it! They're fine as they are.
fetchResult will be automatically typed as `typeof fragment` Co-authored-by: Ned Schwartz <ned@theinterned.net>
|
Thanks again for your time ! It was great collaborating with you 😄 |
|
This has been released https://github.com/github/auto-complete-element/releases/tag/v3.3.4 |
3 participants
This commit adds fetchingResult property to AutoCompleteElement;
That property can only be set using script;
If set the property will be call in autocomplete.ts#fetchingResult methods;