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

[Feature request] Rework <teal_module>$server to get teal_data object #904

Closed
gogonzo opened this issue Aug 18, 2023 · 4 comments · Fixed by #924
Closed

[Feature request] Rework <teal_module>$server to get teal_data object #904

gogonzo opened this issue Aug 18, 2023 · 4 comments · Fixed by #924
Assignees
Labels

Comments

@gogonzo
Copy link
Contributor

gogonzo commented Aug 18, 2023

After insightsengineering/teal.data#161 modules should be refactored (all teal.modules.*) to receive teal_data object from teal.data.

Dilemmas to solve:

  • what to do with tdata class in teal which is sent to data argument in teal_module$server? Should we soft-deprecate?
  • Should we enable another object to work with data_merge and data_extract module which is a candidate to refactor?
@donyunardi
Copy link
Contributor

donyunardi commented Sep 21, 2023

Acceptance Criteria:

  • Make sure the new tdata object (extension of qenv class) being pass to the modules
  • Assess and update how the module received the new tdata
  • Assess if the data_merge and _extract behaves appropriately, fix if not.
  • Check roxygen note if any, or other documentation

We'll start with one module before hitting the rest.

On deprecation topic, we have to think about users who created their own teal modules for their projects. How this update would affect these users and what we can do about it.

@chlebowa chlebowa self-assigned this Sep 22, 2023
@chlebowa
Copy link
Contributor

chlebowa commented Sep 22, 2023

qenv cannot hold reactive expressions in the code slot so filter panel code cannot be passed down to the module.
Can an S4 expert help me out here?

https://github.com/insightsengineering/teal/blob/main/R/module_nested_tabs.R#L313

@gogonzo
Copy link
Contributor Author

gogonzo commented Sep 25, 2023

qenv cannot hold reactive expressions in the code slot so filter panel code cannot be passed down to the module. Can an S4 expert help me out here?

@chlebowa object passed to the modules should be wrapped in a reactive. To be specific then - modules should get reactive returning tdata.

@chlebowa chlebowa linked a pull request Sep 25, 2023 that will close this issue
@gogonzo gogonzo changed the title [Feature request] Rework <teal_module>$server to get tdata object [Feature request] Rework <teal_module>$server to get teal_data object Oct 6, 2023
@gogonzo
Copy link
Contributor Author

gogonzo commented Oct 18, 2023

Closing in favour of #937

@gogonzo gogonzo closed this as completed Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants