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

Fix for onServerSearch and labelRenderer #666

Merged
merged 7 commits into from
Feb 22, 2024

Conversation

MichalLauer
Copy link
Contributor

This PR fixes issue mentioned in #664 and replaces #665.

I did not use eval(...) as suggested because I found on the internet that it might cause security issues. Because of that, I chose the window[...] approach. One notable drawback of this implementation is that the functions that will be used (passed in the onServerSearch parameter) must be defined outside of any document.ready statement.
I also updated the docs, NEWS.md and version.

Sorry if the news/version update is overboard - feel free to fix it or LMK and I'll create a new PR :)

Thanks!

@pvictor
Copy link
Member

pvictor commented Feb 21, 2024

Thanks for this.
Can you not change version number ? Or just the 9000 part, version 0.8.2 hasn't been released on CRAN yet.
Can you put an exemple in examples/ to demonstrate how to use onServerSearch ?

@MichalLauer
Copy link
Contributor Author

Hello @pvictor,

I added example into the examples/ folder and referenced it in the R/virtual-select.R. I also reverted back the version.
If anything else is needed, please, let me know :)

@pvictor pvictor merged commit 3deca87 into dreamRs:master Feb 22, 2024
7 checks passed
@pvictor
Copy link
Member

pvictor commented Feb 22, 2024

That's perfect, thank you! I will release on CRAN shortly.

@MichalLauer MichalLauer deleted the fix-onServerSearch branch February 22, 2024 12:27
@MichalLauer
Copy link
Contributor Author

Awesome, thanks for the collaboration :)

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.

2 participants