Re: [patch][pgAdmin] RM7018 [React] Port Restore dialog to React.

Thanks, the patch applied.

On Sun, Dec 26, 2021 at 4:18 PM Rahul Shirsat <
rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:

> Hi Aditya/Akshay,
>
> Please find the comments below.
>
> On Wed, Dec 22, 2021 at 1:57 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Rahul,
>>
>> Everything looks good except:
>>
>> 1. Remove unnecessary whitespaces
>>
>>
>> adityatoshniwal(at)LAPTOP381PNIN pgadmin4_copy % git apply
>> ~/Downloads/RM7018.patch
>>
>> /Users/adityatoshniwal/Downloads/RM7018.patch:14070: trailing whitespace.
>>
>> constructor(getRestoreSectionSchema, getRestoreTypeObjSchema,
>> getRestoreSaveOptSchema,
>>
>> /Users/adityatoshniwal/Downloads/RM7018.patch:15563: trailing whitespace.
>>
>> import RestoreSchema, {getRestoreSaveOptSchema,
>> getRestoreQueryOptionSchema, getRestoreDisableOptionSchema,
>>
>> /Users/adityatoshniwal/Downloads/RM7018.patch:15620: new blank line at
>> EOF.
>>
>> +
>>
>> warning: 3 lines add whitespace errors.
>>
> Done
>
>>
>> 2. Use lodash, and not underscore.
>>
>> + 'sources/gettext', 'sources/url_for', 'underscore', 'pgadmin.browser',
>> 'sources/utils',
>>
> It's removed now.
>
>> 3. Use gettext
>> panel.title(`Restore (${pgBrowser.Nodes[data._type].label}:
>> ${data.label})`);
>>
> It's fixed now.
>
>> 4. Dialog size is huge. Reduce the dialog size.
>>
> As per discussion on the call, I have reduced the height of the dialog
> (both for restore & backup)
>
>> 5. Mouseover on header should change cursor to "move"
>>
> It's fixed now.
>
>>
>> [image: image.png]
>>
>> 6. If I remove the bin path from preferences, the "utility not found"
>> error comes after I click on restore. It should come as soon as I hit the
>> menu. Same problem with backup.
>>
> Fixed for both restore & backup utility.
>
>> 7. In restore.js the below code is repeated. You should get the treeInfo
>> only once the dialog opens.
>> const selectedNode = pgBrowser.tree.selected();
>> var selectedTreeNode = pgBrowser.tree.findNodeByDomElement(selectedNode);
>> const treeInfo = pgBrowser.tree.getTreeNodeHierarchy(selectedTreeNode);
>>
> Refactored the code, it's removed now.
>
>> 8. Minor spacing fixes between arrow and text.
>> ()=>getRestoreQueryOptionSchema({nodeInfo: treeNodeInfo}),
>> ()=> getRestoreDisableOptionSchema({nodeInfo: treeNodeInfo}),
>> ()=> getRestoreMiscellaneousSchema({nodeInfo: treeNodeInfo}),
>>
> Have taken care of this.
>
>>
>> On Wed, Dec 22, 2021 at 11:35 AM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Aditya
>>>
>>> Can you please review it?
>>>
>>> On Tue, Dec 21, 2021 at 4:52 PM Rahul Shirsat <
>>> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> Please find the attached patch, which ports the restore dialog to react.
>>>>
>>>> --
>>>> *Rahul Shirsat*
>>>> Senior Software Engineer | EnterpriseDB Corporation.
>>>>
>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>> *pgAdmin Hacker | Principal Software Architect*
>>> *EDB Postgres <http://edbpostgres.com>*
>>>
>>> *Mobile: +91 976-788-8246*
>>>
>>
>>
>> --
>> Thanks,
>> Aditya Toshniwal
>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>> <http://edbpostgres.com>
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> *Rahul Shirsat*
> Senior Software Engineer | EnterpriseDB Corporation.
>

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Principal Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*