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

From: Xuri Gong <xurigoong(at)gmail(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com>, 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-25 11:42:25
Message-ID: CAA7HE_fb3_3apPhwMU=khzptYK5GZoPC_SZv=FBN_p8LVB2yKw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

Attached is the newer patch. Please review it. Thank you!

Xuri Gong <xurigoong(at)gmail(dot)com> 于2018年8月19日周日 上午12:14写道:

> Hi Dave,
>
> - The maps sometimes include links to Leaflet or OpenStreet Map. Clicking
>> these will open them in the current Query Tool tab. We need to ensure they
>> open in a new browser tab or window, not inside pgAdmin.
>> - If you press the view button without selecting any rows, you get a
>> message saying "Please select some rows". I think that if no rows are
>> selected, instead of displaying that message, we should ensure the entire
>> resultset is load (as happens if you click the top-left column/row header)
>> and then render all rows.
>
>
> Thank you! I will adjust.
>
> - The dialogue parent seems to be the Query Tool that opened it, which
>> doesn't seem unreasonable, except it means it can only be moved within the
>> confines of the Query Tool panel. Can we make the parent the browser window
>> itself, so the dialogue can be moved anywhere, much like any of the
>> properties dialogues?
>
>
> I failed to find a proper way to do that... However, instead of placing
> the map in Alertify dialogue, we can create a new tab to display the map
> after users click 'view geometry' button. Then users can adjust the layout
> or close the tab as they like(see the attached pictures). I think this
> might be a better design. What's your opinion?
>
>
> Dave Page <dpage(at)pgadmin(dot)org> 于2018年8月16日周四 下午8:11写道:
>
>> Hi
>>
>> That's looking more awesome each time I try it :-). I found just 3 things
>> I think need tweaking:
>>
>> - The maps sometimes include links to Leaflet or OpenStreet Map. Clicking
>> these will open them in the current Query Tool tab. We need to ensure they
>> open in a new browser tab or window, not inside pgAdmin.
>>
>> - The dialogue parent seems to be the Query Tool that opened it, which
>> doesn't seem unreasonable, except it means it can only be moved within the
>> confines of the Query Tool panel. Can we make the parent the browser window
>> itself, so the dialogue can be moved anywhere, much like any of the
>> properties dialogues?
>>
>> - If you press the view button without selecting any rows, you get a
>> message saying "Please select some rows". I think that if no rows are
>> selected, instead of displaying that message, we should ensure the entire
>> resultset is load (as happens if you click the top-left column/row header)
>> and then render all rows.
>>
>> Thanks!
>>
>> On Wed, Aug 15, 2018 at 4:56 PM, Xuri Gong <xurigoong(at)gmail(dot)com> wrote:
>>
>>> Hi,
>>>
>>> I have updated the geometry viewer:
>>>
>>> - Increase geometry column width.
>>> - Remove per-row buttons and just display the row(s) that are
>>> selected.
>>> - Change the viewer panel to regular dialogue.
>>> - Update documents and libraries.txt file.
>>>
>>> Attached are the patch and screenshots. Please review it. Thank you.
>>>
>>> Xuri Gong <xurigoong(at)gmail(dot)com> 于2018年8月11日周六 下午9:23写道:
>>>
>>>> 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
>>>>>>
>>>>>
>>>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

Attachment Content-Type Size
geometry_viewer.diff application/x-patch 1.2 MB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Aditya Toshniwal 2018-08-27 05:39:04 Re: [pgAdmin4][Patch]: RM 1253 - Store and reload current location in treeview
Previous Message Akshay Joshi 2018-08-24 05:24:55 [pgAdmin4][Patch]: RM #3420 Merge latest code of pgcli with pgAdmin4