Re: [pgAdmin4][Patch] - RM 3009 - Right click to copy from data grid, optionally with headers.

On Mon, Sep 23, 2019 at 4:58 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> On Mon, Sep 23, 2019 at 11:03 AM Khushboo Vashi <
> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> Hi,
>>
>> On Mon, Sep 23, 2019 at 3:16 PM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Khushboo
>>>
>>> Following are the review comments:
>>>
>>> - Copy with headers not working when we select a few rows.
>>>
>>> I have considered this option only with the entire grid but will fix
>> this.
>> So, If I select only 2 rows and also select the option Copy with header,
>> then should the header be highlighted? I have highlighted in case of entire
>> grid, with Copy with header option.
>> @Dave, please suggest.
>>
>
> Yes - highlight what will be copied please.
>

I personally think we should not because when user select any row Header
row gets highlighted automatically if "Copy with headers" is checked. Then
we will have to make sure if row gets deselected then header row should not
highlighted.

>
>
>>
>>> - The dropdown should be disabled when the copy button is disabled.
>>>
>>> That is not appropriate. If I have to select the Copy with header option
>> before selecting any row, then not possible with the proposed suggestion.
>>
>
> I agree. No need to disable options that could be toggled at any time.
>

Does it make sense when your main action button is disabled and nearby
drop down is enabled, just because user can toggle them anytime? Anyways
copy button gets enabled when row(s) will be selected and "Copy with
headers" comes in affect when we click on Copy button.

>
>>> - When user select/de-select "Copy with headers" option dropdown
>>> should not be closed. It should be consistent with "Auto Commit/Rollback"
>>> or explain options.
>>> - Feature test "CopySelectedQueryResultsFeatureTest" failed on my
>>> machine.
>>> - Documentation changes are required, update the screenshot wherever
>>> applicable.
>>>
>>> Right, I have already thought of that, but when I checked the current
>> version, I realised that the documentation is not up-to-date with the
>> query-tool toolbar. I didn't find the screen-shots of the many tool-bar
>> dropdown options. So, I would like to involve Abhilasha in this, if
>> everybody agrees, otherwise I can just change the documentation for my
>> patch only.
>>
>
> Yes, please ask Abhilasha to help ensure the docs are properly updated.
>

Generally updating the documentation is the part of the patch itself
unless changes are very big. Most of the time we modified the documentation
and correct whatever is wrong or missing something from the docs.

>
> Thanks.
>
>
>>
>> Thanks,
>> Khushboo
>>
>>>
>>> On Mon, Sep 23, 2019 at 11:42 AM Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please find the attached patch for RM #3009 - Right click to copy from
>>>> data grid, optionally with headers.
>>>>
>>>> Query Tool / View data:
>>>>
>>>> Currently the result-set can be copied without header. With this patch
>>>> the result-set can be copied with the header also and that is optional.
>>>>
>>>> To copy the result-set with header, the option '*Copy with header'* is
>>>> given next with the Copy button in the toolbar in the form of dropdown.
>>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>>
>>> *Sr. Software Architect*
>>> *EnterpriseDB Software India Private Limited*
>>> *Mobile: +91 976-788-8246*
>>>
>>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
*Thanks & Regards*
*Akshay Joshi*

*Sr. Software Architect*
*EnterpriseDB Software India Private Limited*
*Mobile: +91 976-788-8246*