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

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

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 Dave Page 2018-08-10 12:51:35 pgAdmin 4 commit: Handle connection errors properly in the query tool.
Previous Message Akshay Joshi 2018-08-09 13:43:32 pgAdmin 4 commit: Tag REL-3_2 has been created.