Re: [patch][pgAdmin] RM7018 [React] Port Restore dialog to React. - Mailing list pgadmin-hackers

Thanks, the patch applied.

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

Hi Aditya/Akshay,

Please find the comments below.

On Wed, Dec 22, 2021 at 1:57 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Rahul,

Everything looks good except:

1. Remove unnecessary whitespaces

adityatoshniwal@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. 

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.joshi@enterprisedb.com> wrote:

Hi Aditya

Can you please review it?

On Tue, Dec 21, 2021 at 4:52 PM Rahul Shirsat <rahul.shirsat@enterprisedb.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

Mobile: +91 976-788-8246

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | 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

Mobile: +91 976-788-8246