Hi All Added support of code-folding for QueryTool. Please ignore previous patch, attached is the new combined patch.
On Thu, Apr 21, 2016 at 2:00 PM, Akshay Joshi <akshay.jo...@enterprisedb.com
> wrote:
> Hi
>
> I have fixed following review comments given by Dave and attached is the
> patch file:
>
> - The button bar be moved out into an HTML template
> - create.sql should perhaps be renamed to insert.sql
> - The "Add Row" button only works if you're on the last page of the
> resultset.
>
> I have tried to fix the following review comments, but unable to figure
> out the solution
>
>
>
> - Please add an SQL button. This should show/hide the SQL panel in
> *both* Query Tool and Edit Grid modes.
>
> I have changed the design and have only one wcDocker and add
> SQL panel to the Top and remaining panel to the Bottom, then I have tried
> to hide the top panel, but not able to fix it.
>
> - If a field has been edited, but not saved, can we highlight it
> somehow? Maybe make the text dark blue?
>
> For this we have "backgrid:edited" event in which we get model
> and column, but not sure how we can find the cell to add the class to
> change the colour and if we will have solution then, we will have to remove
> that class again once user will enter the original value again.
>
> - Can the "Copy Row" button also populate the clipboard with CSV data
> for the row?
> - Cut, Copy and Paste to Clipboard.
>
> I have googled for solution and found some of them which have
> only "Copy" feature, some of them works for limited browsers. One solution
> is to use "document.execCommand("Copy")" but it works for TextArea and we
> have CodeMirror.
>
> - The layout of the result tabs should be maintained if new Query Tool
> or Edit Grid tabs are opened.
>
>
>
>
>
>
>
>
>
>
>
> To solve the above I need some help/suggestions here.
>
>
> On Fri, Apr 15, 2016 at 1:22 PM, Dave Page <dave.p...@enterprisedb.com>
> wrote:
>
>> Thanks - committed.
>>
>> On Fri, Apr 15, 2016 at 8:03 AM, Akshay Joshi <
>> akshay.jo...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Thu, Apr 14, 2016 at 7:40 PM, Dave Page <dave.p...@enterprisedb.com>
>>> wrote:
>>>
>>>> Hi
>>>>
>>>> On Thu, Apr 14, 2016 at 1:58 PM, Akshay Joshi <
>>>> akshay.jo...@enterprisedb.com> wrote:
>>>>
>>>>> Hi All
>>>>>
>>>>> I have fixed review comments given by Dave and couple of them are
>>>>> remaining
>>>>>
>>>>> *Fixed Review Comments:*
>>>>>
>>>>> - The View Data menu option should be on the Object menu, which
>>>>> should mirror the Context menu, except options should be disabled when
>>>>> not
>>>>> applicable instead of hidden.
>>>>> - The Query Tool menu icon should be a glyphicon, to match the
>>>>> others.
>>>>> - Please merge the functionality of the Refresh and Execute
>>>>> buttons into one button. We shouldn't have two buttons that do
>>>>> essentially
>>>>> the same thing.
>>>>> - In Edit Grid mode, the History panel should log all queries (
>>>>> SELECTs, UPDATEs, DELETEs etc) as it would in the Query Tool.
>>>>> - In Edit Grid mode, the Messages panel should display any
>>>>> messages from the most recent action as it would in the Query Tool.
>>>>> - In Edit Grid mode, that textbox should be read-only, but should
>>>>> display the SQL used (including any LIMIT/FILTER clauses)
>>>>> - Please remove the border from the SQL box, such that it fills
>>>>> all available space.
>>>>> - The Filter box should be in a modal overlay over the top of the
>>>>> SQL box/Results tabs as required. Those elements should be grayed
>>>>> whilst it is open.
>>>>> - Please adjust the height of the Delete icon in the Edit Grid,
>>>>> such that it doesn't force the row height to be higher than it should
>>>>> be.
>>>>> - I think the names of the tabs are far too long. Can we change
>>>>> them to "Query 1", "Query 2" etc, then rename them to the filename
>>>>> if the user saves/loads a file?
>>>>> - We should add an "Edit" button, which opens a drop-down menu.
>>>>> This would eventually include options as found on the Edit menu in the
>>>>> pgAdmin3 query tool, such as the "Clear SQL" option.
>>>>> - Ashesh and I discussed changing the History tab to be a grid,
>>>>> showing: Date/Time, Query (first line only), Rows affected, Runtime
>>>>> and Status, in a row per query executed. Ashesh suggested using a
>>>>> sub-form that can be expanded for each row, which could show the full
>>>>> query
>>>>> and error details (SQL State etc). New rows should be added to the
>>>>> top of the list.
>>>>> - Errors should be highlighted in the SQL box - a marker in the
>>>>> margin to note the line, and spellcheck-style underlining for the error
>>>>> word.
>>>>> - Query results should have spaces converted to " ", so that
>>>>> proper indenting is maintained (for example, on EXPLAIN queries).
>>>>> - on shutdown pgAdmin server , appropriate message should be
>>>>> displayed.
>>>>>
>>>>>
>>>> Awesome!
>>>>
>>>> I've made a few minor style changes - mostly colouring, but I also
>>>> widened the Execute button and it was easier to push it's dropdown than it
>>>> - and pushed the code.
>>>>
>>>>
>>>>> *Remaining review comments*:
>>>>>
>>>>> - Please add an SQL button. This should show/hide the SQL panel in
>>>>> *both* Query Tool and Edit Grid modes.
>>>>> - If a field has been edited, but not saved, can we highlight it
>>>>> somehow? Maybe make the text dark blue?
>>>>> - The "Add Row" button only works if you're on the last page of
>>>>> the resultset.
>>>>> - Can the "Copy Row" button also populate the clipboard with CSV
>>>>> data for the row?
>>>>> - In Edit mode, we need to be able to represent/set values to NULL.
>>>>> - The layout of the result tabs should be maintained if new Query
>>>>> Tool or Edit Grid tabs are opened.
>>>>>
>>>>> *TODO's* (apart from above)
>>>>>
>>>>> - Open/Save SQL file.
>>>>> - Cut, Copy, Paste functionality.
>>>>>
>>>>> Agreed on the above.
>>>>
>>>> My only real suggestion on the code itself (which looks good and clean
>>>> on a quick review), is that:
>>>>
>>>> - The button bar be moved out into an HTML template
>>>> - create.sql should perhaps be renamed to insert.sql
>>>> - It looks like we only allow updates if we have a pkey. Should we
>>>> allow use of OIDs when present, if a pkey isn't there?
>>>>
>>>> I'll continue to log additional issues if/when I find them.
>>>>
>>>
>>> Thanks for committing the patch. I'll work on the above comments.
>>> Meanwhile I have found one issue where "View Filtered Row" is not working,
>>> so attached is the patch file to fix that. Can you please review/commit it.
>>>
>>>>
>>>> --
>>>> Dave Page
>>>> VP, Chief Architect, Tools & Installers
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>
>>>
>>>
>>> --
>>> *Akshay Joshi*
>>> *Principal Software Engineer *
>>>
>>>
>>>
>>> *Phone: +91 20-3058-9517 <%2B91%2020-3058-9517>Mobile: +91 976-788-8246*
>>>
>>
>>
>>
>> --
>> Dave Page
>> VP, Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>
>
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>
--
*Akshay Joshi*
*Principal Software Engineer *
*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
Fixed_review_comments_v2.patch
Description: Binary data
-- Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
