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

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Rahul Shirsat <rahul(dot)shirsat(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Subject: Re: [patch][pgAdmin] RM7018 [React] Port Restore dialog to React.
Date: 2021-12-22 08:27:07
Message-ID: CAM9w-_n6nC9+6gS+kxgWiWD=GYTEhwadk8i1gKJOEOB+RTunuA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.

2. Use lodash, and not underscore.

+ 'sources/gettext', 'sources/url_for', 'underscore', 'pgadmin.browser',
'sources/utils',
3. Use gettext
panel.title(`Restore (${pgBrowser.Nodes[data._type].label}:
${data.label})`);
4. Dialog size is huge. Reduce the dialog size.
5. Mouseover on header should change cursor to "move"

[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.
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);
8. Minor spacing fixes between arrow and text.
()=>getRestoreQueryOptionSchema({nodeInfo: treeNodeInfo}),
()=> getRestoreDisableOptionSchema({nodeInfo: treeNodeInfo}),
()=> getRestoreMiscellaneousSchema({nodeInfo: treeNodeInfo}),

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"

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2021-12-22 08:55:14 pgAdmin 4 commit: Added beforeNext and beforeBack method in Wizard.
Previous Message Akshay Joshi 2021-12-22 06:05:36 Re: [patch][pgAdmin] RM7018 [React] Port Restore dialog to React.