Re: [pgAdmin4][patch]: Feature #1407 - Support map view for PostGIS query result sets

From: Xuri Gong <xurigoong(at)gmail(dot)com>
To: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>
Cc: Dave Page <dpage(at)pgadmin(dot)org>, pgadmin-hackers(at)postgresql(dot)org, aditya(dot)toshniwal(at)enterprisedb(dot)com
Subject: Re: [pgAdmin4][patch]: Feature #1407 - Support map view for PostGIS query result sets
Date: 2018-08-11 13:23:21
Message-ID: CAA7HE_fGAeWk_dw18R7HwbYa2FeE_Hjyd1M95u7YdOF75uYGLQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave and Khushboo,

Thank you very much for the suggestions and instructions. I will do that
and send the new patch soon.

Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> 于2018年8月10日周五 下午1:15写道:

> Hi Xuri,
>
> On Thu, Aug 9, 2018 at 3:52 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Wed, Aug 8, 2018 at 5:32 PM, Xuri Gong <xurigoong(at)gmail(dot)com> wrote:
>>
>>> Hi Dave
>>>
>>> Thanks for your feedback!
>>>
>>> 1) The column headers should be resized to take into account the button
>>>> size. It looks a little weird when they obscure the text.
>>>>
>>> I will resize the column and update the code style.
>>>
>>
>> Thanks.
>>
>>
>>>
>>> 2) It would be good to be able to select a subset of rows. The grid
>>>> already supports this... I'd suggest that the header button should display
>>>> only the row(s) that are currently selected. That would also allow the
>>>> removal of the per-row buttons.
>>>>
>>> Thanks for your nice idea! This can be implemented without much change
>>> in the current code. I am wondering if it is intuitive enough for users to
>>> firstly select row(s) and then click the column header button. What do you
>>> think?
>>>
>>
>> I think it will be. It's exactly how other operations work (e.g. Copy).
>> They affect the whole grid, unless a subset is selected in which case they
>> only affect that subset.
>>
>>
>>>
>>> 3) Can the panel be made into a regular dialogue, with a close button
>>>> and ability to resize and move it?
>>>
>>> Now the viewer can be resized by dragging the bottom right corner and it
>>> can't be moved. I tried adding frame but found it look not good(see the
>>> attached screenshot).
>>>
>>
>> Hmm. I think that is an improvement, but it needs to lose the
>> status/button bar at the bottom and render the map over the entire canvas
>> except the title bar. Oh, and of course, making it movable is important so
>> users can resize and move it so they can see other things underneath it.
>>
>> Khushboo/Aditya - you've worked with the dialogues much more recently
>> than I; can you give Xuri some pointers on how to do the above?
>>
>> To make the Alertify dialogue movable you can set movable to true in the
> options of the setup function of Alertify.
> For changing the footer, you can use this.elements.footer in the build
> function which will give you the footer div. So you can apply the css to
> hide it.
>
> To change the size of the dialogue, resizeTo function is available. So
> instead of writing elements.dialogue.style.height / weight, you can use
> this function also.
>
> Whenever we introduce a new JS library, we maintain the list in
> libraries.txt file. So you need to add the 'wkx' library to this file.
>
> Please let me know if you have any doubts.
>
> Thanks,
> Khushboo
>
>> Thanks!
>>
>>
>>>
>>>
>>> Dave Page <dpage(at)pgadmin(dot)org> 于2018年8月7日周二 下午11:41写道:
>>>
>>>> Hi
>>>>
>>>> On Sat, Aug 4, 2018 at 1:27 PM, Xuri Gong <xurigoong(at)gmail(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I have added documents for geometry viewer. Please review this newer
>>>>> patch.
>>>>>
>>>>> For test you can install PostGIS and import this database[1].
>>>>>
>>>>> Thank you!
>>>>>
>>>>> [1] https://drive.google.com/open?id=1NHWW4WPli7kxpuGaDFGVFLDXdc2D20wp
>>>>>
>>>>
>>>> I finally got a chance to play with this! It's pretty awesome - I think
>>>> some tweaks are needed to polish it off though:
>>>>
>>>> 1) The column headers should be resized to take into account the button
>>>> size. It looks a little weird when they obscure the text.
>>>>
>>>> 2) It would be good to be able to select a subset of rows. The grid
>>>> already supports this... I'd suggest that the header button should display
>>>> only the row(s) that are currently selected. That would also allow the
>>>> removal of the per-row buttons.
>>>>
>>>> 3) Can the panel be made into a regular dialogue, with a close button
>>>> and ability to resize and move it?
>>>>
>>>> I haven't looked at the code in any great depth due to time
>>>> constraints, but I didn't see anything particularly untoward except some of
>>>> the formatting; for example:
>>>>
>>>> if(command === 'view-all-geometries'){
>>>> ...
>>>> }else{
>>>>
>>>> Should look more like:
>>>>
>>>> if (command === 'view-all-geometries') {
>>>> ...
>>>> } else {
>>>>
>>>> Thanks - that's good work!
>>>>
>>>>
>>>>>
>>>>> Xuri Gong <xurigoong(at)gmail(dot)com> 于2018年7月31日周二 下午4:47写道:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> This is the patch for viewing geometry data in pgAdmin4. I have
>>>>>> implemented it as follows:
>>>>>>
>>>>>> - Add cell button and column header button in datagrid for
>>>>>> geometry and geography type data. [screenshot 1]
>>>>>> - When user clicks the 'view' button, it parses the data and
>>>>>> opens a map to show the geometry. [screenshot 2, 3, 4]
>>>>>> - When user clicks the geometry in the map, it shows the
>>>>>> properties. [screenshot 5]
>>>>>>
>>>>>> I have created unit test for this. I have also created a test
>>>>>> database[1] including geometries of different types to run test manually.
>>>>>> Please review it. Thank you.
>>>>>>
>>>>>> Regards,
>>>>>> Xuri Gong
>>>>>>
>>>>>> [1]
>>>>>> https://drive.google.com/open?id=1NHWW4WPli7kxpuGaDFGVFLDXdc2D20wp
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2018-08-12 10:15:33 pgAdmin 4 commit: We were refering to the wrong 'name' variable during
Previous Message Dave Page 2018-08-10 12:52:07 Re: [pgAdmin4][Patch]: RM #3528 Query Tool doesn't handle 'Too many connection' error properly