I liked the way you have refactored the code at some places in the JS file and made it cleaner.
Here are some points:
1. The table (including partition table) with a single column having that column primary key is editable but the save button is disabled, so, ultimately I can't save the data. Note: The table should be empty to reproduce this issue.
I tried to reproduce the issue but it works fine for me. Please make sure that you edit the empty cell (to add a new row) and press enter for the edits on the grid to take effect.
Right, on the enter, the button gets enabled, not on the focus out, so this is by design, not something your patch caused.
2. command.py - The check_updatable_results_pkeys function calling the poll function and checks the ASYNC_OK, I think this is not required as this function is called from the poll function from the sqleditor/__init__.py *if the status of the polling is if ASYNC_OK*. So, I think this is overhead but if you have considered another scenario then let me know.
It was just a sanity check, just in case someone calls the function incorrectly. I removed it and added comments below the function header indicating when it should be called.
Please make sure to remove "from pgadmin.tools.sqleditor.utils.constant_definition import ASYNC_OK" line from the file as not required anymore.
3. In the Preferences, the label of the keyboard shortcut "Save Data Changes" should be "Save data changes".
Corrected.
4. Dave has already mentioned about the commented code, so I do agree we should remove it.
Removed.
5. I didn't find the doc updates for the keyboard shortcuts in the Preferences module as well as related to this feature. Am I missing something?
I don't know which part exactly are you referring to.
In the previous patch, I have already updated keyboard_shortcuts.rst to document the new button shortcut, in addition to preferences.rst to document the new 'Alert on uncommited changes' option. I also updated query_tool.rst, query_tool_toolbar.rst & editgrid.rst to document the new features. Which part is missing?
My bad, I have applied the patch on the web folder, so the difference of rst files didn't get applied. Now, I can see the doc updates and it looks good to me.
