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

Hi Akshay,

On Mon, Sep 23, 2019 at 4:10 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

>
>
> 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.
>

If we highlight the header row for a selection of all rows, then we should
do the same for a subset (and if it's a subset of columns, highlight only
those that are selected of course).

>
>>
>>>
>>>> - 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.
>

Yes. I can change other copy related options in the Preferences panel when
the copy button is disabled. I don't think is this different, except we're
putting it on a menu for convenience.

>
>
>>
>>>> - 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.
>

Yes, but as Khushboo pointed out, we've missed some updates already.
Therefore Abhilasha can help sort that out :-)

>
>
>>
>> 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*
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company