Hi Yosry,
Nice work there. It seems to be working fine except a few suggestions:
1) Fix pep8 issues
2) DOM Statements like below can be avoided and html can be added directly
to main template of $el instead of adding extra operations of find, prepend
and append. Plus, it makes it difficult to understand what will the DOM
look like.
this.$el.find('.query').prepend('<i id="query_source_icon"
class="query-history-icon sql-icon-lg"></i>');
$container.append($toggleEl).append(self.$el);
3) Change the below to use class d-none with toggleClass('d-none') for
consistency across.
this.$el.find('.pgadmin-query-history-entry').each(function() {
$(this).toggle();
});
4) I may be wrong, but I'm seeing the flash icon for view/edit data queries
and view table icon for query tool queries. Looks like they are swapped.
[image: Screenshot 2019-08-12 at 12.15.18.png]
On Sun, Aug 11, 2019 at 11:58 AM Yosry Muhammad <yosry...@gmail.com> wrote:
> Hi Hackers,
>
> Please find attached a patch including new features for the Query History
> as follows:
>
> - Integration with updatable resultsets: Queries generated by pgAdmin
> during Save Data operations are now shown in the history panel, including
> transaction control queries.
> - Source indication: The source of each query in the history panel is
> indicated using an icon corresponding to the toolbar (Save Data, Execute,
> Explain, Commit, Rollback & View/Edit).
> - Allow showing/hiding queries generated internally by pgAdmin using a
> toggle button.
> - Some refactoring in save_data_changes.py
>
> The patch includes the code for the feature, added feature tests and
> documentation updates.
> Please review !
>
> Thanks. Regards.
>
> --
> *Yosry Muhammad Yosry*
>
> Computer Engineering student,
> The Faculty of Engineering,
> Cairo University (2021).
> Class representative of CMP 2021.
> https://www.linkedin.com/in/yosrym93/
>
--
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB India | Pune
"Don't Complain about Heat, Plant a TREE"
