-
-
Notifications
You must be signed in to change notification settings - Fork 902
Move aggrid temporarily to read data despite unmounted #5615
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
Conversation
0889fc4 to
1adacbd
Compare
|
Note: It may be desirable to have |
|
Actually is this PR a good idea? The point of Maybe we should modify the component to emit the data before unmount instead. That would be more practical. |
|
We need the new functionality because the user-edited data persists in the hidden tab. The element may be not unmounted but just hidden from DOM? Anyways just minor terms issue. from nicegui import ui
with ui.tabs().classes('w-full') as tabs:
one = ui.tab('One')
two = ui.tab('Two')
with ui.tab_panels(tabs, value=one).classes('w-full'):
with ui.tab_panel(one):
async def get_data():
ui.notify(await grid.get_client_data())
ui.button('Get data (offscreen)', on_click=get_data)
with ui.tab_panel(two):
grid = ui.aggrid({
'columnDefs': [{'headerName': 'Name', 'field': 'name', 'editable': True}],
'rowData': [{'name': 'Alice'}, {'name': 'Bob'}],
})
ui.button('Get data (onscreen)', on_click=get_data)
ui.run()
|
|
what would happen if we store the API object of the element somewhere in window? This way we would not need to swap-in the element just for the API object Or, is there already some reference stored somewhere to allow me to access it? |
So something like result = await self.client.run_javascript(f'''
const rowData = [];
__nicegui_aggrid_api__["{self.html_id}"].{API_METHODS[method]}(node => rowData.push(node.data));
return rowData;
''', timeout=timeout) this.api = AgGrid.createGrid(this.$el, this.gridOptions);
if (window.__nicegui_aggrid_api__ === undefined) window.__nicegui_aggrid_api__ = {};
window.__nicegui_aggrid_api__[this.$el.id] = this.api;It works, but the AG Grid must be already rendered once.
Considering the use case of fetching user-edited data from AG Grid, it may be advisable to just let @falkoschindler We should carefully consider this before merging the PR because we don't want to enable something at the cost of poor code decisions. |
|
Thanks @evnchn! This is an interesting approach. But it feels a bit fragile:
|
|
Exactly Hence why in #5615 (comment) I recommend against doing this element-swap trick, and just store the API object to enable mounted elements to read the API off-screen while unmounted elements remain inaccessible |
|
Ok, which brings us to #5612 (review): a client-side memory leak, unless we make sure to prune the storage when the element is deleted. |
|
Well then, I am thinking, instead of all elements writing to |
|
Let's discuss the broader picture over at #3033 (comment). |
|
I don't like this anymore. You will know why shortly. |
Passing on local
Motivation
Fix #3033, in which we find it is not possible to read data off client-side AGGrid if it is unmounted.
Implementation
This is a decent approach:
client.contentto force it to renderNow that we are doing it in the client-side, stuff is much better (check before force-push to see what I did 🤡)
Progress