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

From: Rahul Shirsat <rahul(dot)shirsat(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [patch][pgAdmin] RM7018 [React] Port Restore dialog to React.
Date: 2022-01-06 15:05:53
Message-ID: CAKtn9dOzzgNDq3OKDYcihHjFM5Rx2bfz=nMOZ++oOtwor29psg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Akshay,

Please find the updated screenshots and descriptions for below dialogs:

1. Restore Dialog
2. Backup Dialog
3. Backup Server Dialog
4. Backup Globals Dialog

On Thu, Jan 6, 2022 at 4:49 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Rahul
>
> Patch applied with height and width changes to your patch.
> Please send an updated documentation patch with updated screenshots and
> descriptions.
>
> On Thu, Jan 6, 2022 at 2:11 PM Rahul Shirsat <
> rahul(dot)shirsat(at)enterprisedb(dot)com> wrote:
>
>> Hi Hackers,
>>
>> Please find the updated patch v3, which includes the fixes of -
>>
>> 1. Split the restore options / backup options into 2 tabs
>> 2. Restore icon changes
>> 3. feature tests issues
>> 4. restore help link broken
>> 5. Comment section not visible
>> 6. only data for table - should be yes and disabled
>>
>>
>> On Tue, Dec 28, 2021 at 1:50 PM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> 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*
>>>
>>
>>
>> --
>> *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*
>

--
*Rahul Shirsat*
Senior Software Engineer | EnterpriseDB Corporation.

Attachment Content-Type Size
RM7018_v4.patch application/octet-stream 4.0 MB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2022-01-07 06:38:53 pgAdmin 4 commit: Updated screenshots and descriptions for Backup and R
Previous Message Akshay Joshi 2022-01-06 14:31:35 pgAdmin 4 commit: Fixed import/export server issues reported while test