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

Hi Dave,

Will do, thank you very much!

Dave Page <dpage(at)pgadmin(dot)org> 于2018年8月30日周四 下午9:13写道：

> Hi
>
> On Sat, Aug 25, 2018 at 12:42 PM, Xuri Gong <xurigoong(at)gmail(dot)com> wrote:
>
>> Hi Dave,
>>
>> Attached is the newer patch. Please review it. Thank you!
>>
>
> That's awesome! I've applied the patch with a few minor changes to the
> documentation and the attribution notices (for consistency with each other,
> and to remove the unnecessary leaflet link).
>
> Although I've committed this (I wanted to get it into 3.3 which will be
> built next week), there are still some final fixes to be made - can you
> please review all the strings in the JS code, and ensure they are properly
> wrapped in gettext calls? Some strings are not localised at all, and a
> couple are built using constructs like str = gettext("There were ") + num +
> gettext(" geometries rendered") which won't work with some languages.
>
> Thanks again - great work!!
>
>
>
>>
>> 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
>>>>
>>>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>