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

Remove access to data from UI modules #132

Closed
gogonzo opened this issue Nov 24, 2022 · 2 comments
Closed

Remove access to data from UI modules #132

gogonzo opened this issue Nov 24, 2022 · 2 comments
Assignees
Labels

Comments

@gogonzo
Copy link
Contributor

gogonzo commented Nov 24, 2022

Reason of this issue is that filter-panel UI can't be initialized without data. It's because filter_panel_ui is a part of FIlteredData class. We would like to initialize empty filter-panel and update it when the data is available.

Proposition

UI modules have access to data to initialize the inputs which messes up teal reactivity. Please do:

  • FIlteredData$new() should initialize empty FilteredData with empty ui/srv_filter_panel
  • FilteredData$set_filtered_dataset should create FilteredDataset and insert it into private$filtered_datasets (reactiveValues).
  • All functions which refer to filtered_datasets will be then reactive on a change in reactiveValues
  • UI should not have any connection to the data nor FilteredData (they shouldn't be the arguments of the function)
  • UI should be initialized on the informations we have before data is available - datanames and maybe join keys
  • The rest of the UI components should be either updated or rendered by server when reactive data is available (or changed).

This issue will require other actions which should be addressed in the same PR:

  • Possibly we would like to exclude shiny modules from the classes.
  • move calling UIs and servers to the module_nested_tabs. Feasible - please see the branch
@gogonzo gogonzo added this to the Module specific filter-panel milestone Nov 24, 2022
@gogonzo gogonzo added the core label Nov 24, 2022
@gogonzo gogonzo changed the title Remove accesss to data from UI modules Remove access to data from UI modules Nov 24, 2022
@BLAZEWIM BLAZEWIM self-assigned this Jan 13, 2023
@gogonzo gogonzo added the Blocked label Feb 3, 2023
@gogonzo
Copy link
Contributor Author

gogonzo commented Feb 7, 2023

I suggest to not address this issue at this moment (february 2023). PR I've made #185 shows how complex these modules are. Removing UI from the classes is confusing and not relevant at this moment of the refactor process. Issue is related more with the teal 1.0 milestone

@gogonzo
Copy link
Contributor Author

gogonzo commented May 22, 2024

Closing this up as it can be done under this issue insightsengineering/teal#669

@gogonzo gogonzo closed this as completed May 22, 2024
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