From: | Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com> |
---|---|
To: | Yosry Muhammad <yosrym93(at)gmail(dot)com> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Dave Page <dpage(at)pgadmin(dot)org> |
Subject: | Re: [GSoC] Query History Upgrade |
Date: | 2019-08-12 07:03:33 |
Message-ID: | CAM9w-_mAnPo+5pXjbk3rZu0Wi8NL97sF6NBiOYCkxRmXH2p9gQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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 <yosrym93(at)gmail(dot)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"
From | Date | Subject | |
---|---|---|---|
Next Message | Akshay Joshi | 2019-08-12 07:42:05 | pgAdmin 4 commit: Fix issue in drag drop feature. It's regression of RM |
Previous Message | Aditya Toshniwal | 2019-08-12 06:51:40 | Re: [pgAdmin][RM4552] Dragging the selected text in SQL editor throws console errors |