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

Added functions to get client data to AgGrid element #1079

Merged
merged 4 commits into from
Jul 16, 2023

Conversation

BrianLandry
Copy link
Contributor

When the client edits a cell in AgGrid the data is not automatically synced to the python dictionary backing the AgGrid element. To obtain that data this PR adds a helper function to the AgGrid element that executes javascript to obtain the rowData present on the client.

This PR would address this question from Discord and also is something I am using in my own work.

I plan to add tests, but am struggling to get Selenium working right now. I wanted to open this PR in the meantime to make sure there was interest in this feature. If so I'll work on getting the tests together.

@falkoschindler
Copy link
Contributor

@BrianLandry Thanks a lot for this pull request! I finally found the time to have a look into it.

The idea sounds very reasonable. Unfortunately, however, it doesn't seem to work. This is what I'm experimenting with:

grid = ui.aggrid({
    'columnDefs': [{'headerName': 'Name', 'field': 'name', 'editable': True}],
    'rowData': [{'name': 'Alice'}, {'name': 'Bob'}, {'name': 'Carol'}],
})

async def get_data():
    print(await grid.get_client_data())

ui.button('Get data', on_click=get_data)

But it always prints the original data, ignoring any edits.

@BrianLandry
Copy link
Contributor Author

BrianLandry commented Jul 7, 2023

Interesting. I’ll try to dive in this weekend and see whats up. I am using this code regularly and it’s definitely working. So its either a issue with how i moved it over to this code base. Or some more tricky setup specific bug.

@falkoschindler falkoschindler added the enhancement New feature or request label Jul 8, 2023
@BrianLandry
Copy link
Contributor Author

If I edit a cell, and then click the "Get Data" button I replicate your behavior. Hopefully, this is what was occurring on your end. This is because by default the AgGrid has the parameter stopEditingWhenCellsLoseFocus (docs) set as False. This means the cell is still being edited, and the client doesn't have the updated value yet. This is an annoying default I don't expect people to find, and I think the behavior described here will be common and should be addressed.

To address this I can think of three options:

  1. Keep current behavior and add a suggestion to change stopEditingWhenCellsLoseFocus to true in the comments of get_client_data.

  2. Do # 1, but also check the value of stopEditingWhenCellsLoseFocus and if it is false log a warning describing this behavior but continue.

  3. Add getElement({self.id}).gridOptions.api.stopEditing(); (docs) to the javascript in get_client_data. This fixes the issue, but seems to be working around a feature option already in AgGrid, when instead the user should just set the AgGrid option correctly.

I'm leaning towards # 1. I don't think niceGUI should try to address bad defaults in AgGrid, even if it is causing a painpoint when using niceGUI, but it would be great to hear your thoughts.

@falkoschindler
Copy link
Contributor

Thanks for the clarification, @BrianLandry! That might have been the problem. I added a note about stopEditingWhenCellsLoseFocus to the documentation.
Now everything works as expected. Let's merge it! 🙂

@falkoschindler falkoschindler marked this pull request as ready for review July 16, 2023 19:13
@falkoschindler falkoschindler merged commit 48c964f into zauberzeug:main Jul 16, 2023
1 check passed
@falkoschindler falkoschindler added this to the 1.3.4 milestone Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants