Re: [pgAdmin][RM7016]: Port Dependent, dependencies, statistics panel to React.

From: Pradip Parkale <pradip(dot)parkale(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: [pgAdmin][RM7016]: Port Dependent, dependencies, statistics panel to React.
Date: 2022-02-02 12:43:43
Message-ID: CAJ9T6SumnxZ1RGgfq2bpcMSNj=sNgfFH5xmdzufLOfaf4KiOgw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Akshay,

Here is the updated patch. I have fixed all issues found during testing.

On Mon, Jan 24, 2022 at 2:14 PM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Thanks, the patch applied.
>
> On Mon, Jan 24, 2022 at 1:34 PM Pradip Parkale <
> pradip(dot)parkale(at)enterprisedb(dot)com> wrote:
>
>> Hi Akshay,
>>
>> Please find the updated patch.
>>
>> On Mon, Jan 24, 2022 at 11:09 AM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Pradip
>>>
>>> Facing error while selecting the Dependencies and Dependent panels.
>>> Please fix and resend the patch.
>>>
>>> On Sun, Jan 23, 2022 at 1:01 PM Pradip Parkale <
>>> pradip(dot)parkale(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Aditya/Akshay,
>>>>
>>>> Please find the updated patch.
>>>>
>>>> On Sun, Jan 23, 2022 at 12:58 PM Pradip Parkale <
>>>> pradip(dot)parkale(at)enterprisedb(dot)com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, Jan 20, 2022 at 10:30 AM Aditya Toshniwal <
>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Adding to the previous mail:
>>>>>>
>>>>>> 1. Header and rows do not match when there is a scrollbar.
>>>>>> [image: image.png]
>>>>>>
>>>>>> 2. Empty box shown.
>>>>>> [image: image.png]
>>>>>>
>>>>>> 3. Please consider https://redmine.postgresql.org/issues/4852 as
>>>>>> well.
>>>>>>
>>>>>>
>>>>>> On Thu, Jan 20, 2022 at 10:14 AM Aditya Toshniwal <
>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Pradip,
>>>>>>>
>>>>>>> The patch looks good to me except for a few minor changes:
>>>>>>> 1. Rename it generateCollectionURL.
>>>>>>>
>>>>>>> +/* It generates the URL based on collection node selected */
>>>>>>>
>>>>>>> +export function generateCollectionNode(item, type) {
>>>>>>>
>>>>>>>
>>>>>>> 2. We have a theme variable for this.
>>>>>>>
>>>>>>> +const useStyles = makeStyles(() => ({
>>>>>>>
>>>>>>> + background: '#ebeef3',
>>>>>>>
>>>>>>> 3. Do not use css classes. Create style in makeStyle.
>>>>>>>
>>>>>>> + <div className="pg-panel-message">
>>>>>>>
>>>>>>> 4. Why are we adjusting heights this way ? I think PgTable should
>>>>>>> auto handle this.
>>>>>>>
>>>>>>> + <PgTable
>>>>>>>
>>>>>>> + className={classes.autoResizer}
>>>>>>>
>>>>>>> + height={window.innerHeight - 450}
>>>>>>>
>>>>>>>
>>>>>>> 5. We have a theme variable for this.
>>>>>>>
>>>>>>> + borderRadius: '6px',
>>>>>>>
>>>>>>>
>>>>>>> 6. I want to understand how 1% was calculated.
>>>>>>>
>>>>>>> + backgroundPosition: '1%',
>>>>>>>
>>>>>>> 7. Fix SonarQube issues of the new code.
>>>>>>>
>>>>>>> On Wed, Jan 19, 2022 at 5:36 PM Pradip Parkale <
>>>>>>> pradip(dot)parkale(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Aditya,
>>>>>>>>
>>>>>>>> Please find the updated patch.
>>>>>>>>
>>>>>>>> On Wed, Jan 5, 2022 at 1:59 PM Aditya Toshniwal <
>>>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi Pradip,
>>>>>>>>>
>>>>>>>>> We're moving towards React and not just replacing the UI HTML and
>>>>>>>>> keeping everything as it is. The current porting does not help when in
>>>>>>>>> future we remove wcDocker :
>>>>>>>>> 1. 'underscore', 'jquery', 'backbone', 'pgadmin.alertifyjs',
>>>>>>>>> 'pgadmin.backgrid' these should not be referred to at all after porting.
>>>>>>>>>
>>>>>>>>> 2. I can still see this code in the files:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> // Defining Backbone Model for Dependencies.
>>>>>>>>> var Model = Backbone.Model.extend({
>>>>>>>>> defaults: {
>>>>>>>>> icon: 'icon-unknown',
>>>>>>>>>
>>>>>>>>> 3. Since we're also removing jQuery - $.ajax should not be used.
>>>>>>>>> $.ajax({
>>>>>>>>> url: url,
>>>>>>>>> type: 'GET',
>>>>>>>>> })
>>>>>>>>>
>>>>>>>>> 4. There is no need for 2 files - dependencies.js
>>>>>>>>> and DependenciesComponent.jsx.
>>>>>>>>>
>>>>>>>>> There should be only one file - Dependencies.jsx.
>>>>>>>>>
>>>>>>>>> 5. There is no need to create Modules for
>>>>>>>>> dependencies. Dependencies should be directly mounted using
>>>>>>>>> (ReactDOM.render) from panel.js - handleVisibility
>>>>>>>>>
>>>>>>>>> 6. All other required logic should go inside - Dependencies.jsx.
>>>>>>>>> Just pass the required info to the Dependencies.jsx component.
>>>>>>>>>
>>>>>>>>> 7. I have not checked the other two panels but I'm assuming that
>>>>>>>>> its done the same way.
>>>>>>>>> 7. The UI does not look good.
>>>>>>>>> [image: Screenshot 2022-01-05 at 1.54.31 PM.png]
>>>>>>>>> [image: Screenshot 2022-01-05 at 1.26.31 PM.png]
>>>>>>>>>
>>>>>>>>> On Wed, Jan 5, 2022 at 9:30 AM Akshay Joshi <
>>>>>>>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Aditya
>>>>>>>>>>
>>>>>>>>>> Can you please review it?
>>>>>>>>>>
>>>>>>>>>> On Wed, 5 Jan, 2022, 9:18 am Pradip Parkale, <
>>>>>>>>>> pradip(dot)parkale(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>
>>>>>>>>>>> Please find the attached patch. I have a ported dependent ,
>>>>>>>>>>> dependencies and statistics panel to React.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Thanks & Regards,
>>>>>>>>>>> Pradip Parkale
>>>>>>>>>>> Software Engineer | EnterpriseDB Corporation
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Thanks,
>>>>>>>>> Aditya Toshniwal
>>>>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>>>>> <http://edbpostgres.com>
>>>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Thanks & Regards,
>>>>>>>> Pradip Parkale
>>>>>>>> Software Engineer | EnterpriseDB Corporation
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Thanks,
>>>>>>> Aditya Toshniwal
>>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>>> <http://edbpostgres.com>
>>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Thanks,
>>>>>> Aditya Toshniwal
>>>>>> pgAdmin Hacker | Software Architect | *edbpostgres.com*
>>>>>> <http://edbpostgres.com>
>>>>>> "Don't Complain about Heat, Plant a TREE"
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thanks & Regards,
>>>>> Pradip Parkale
>>>>> Software Engineer | EnterpriseDB Corporation
>>>>>
>>>>
>>>>
>>>> --
>>>> Thanks & Regards,
>>>> Pradip Parkale
>>>> Software Engineer | EnterpriseDB Corporation
>>>>
>>>
>>>
>>> --
>>> *Thanks & Regards*
>>> *Akshay Joshi*
>>> *pgAdmin Hacker | Principal Software Architect*
>>> *EDB Postgres <http://edbpostgres.com>*
>>>
>>> *Mobile: +91 976-788-8246*
>>>
>>
>>
>> --
>> Thanks & Regards,
>> Pradip Parkale
>> Software Engineer | EnterpriseDB Corporation
>>
>
>
> --
> *Thanks & Regards*
> *Akshay Joshi*
> *pgAdmin Hacker | Principal Software Architect*
> *EDB Postgres <http://edbpostgres.com>*
>
> *Mobile: +91 976-788-8246*
>

--
Thanks & Regards,
Pradip Parkale
Software Engineer | EnterpriseDB Corporation

Attachment Content-Type Size
RM7016_v5.patch application/octet-stream 10.4 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2022-02-03 05:43:52 pgAdmin 4 commit: Fixed issues related to porting of dependent, depende
Previous Message Akshay Joshi 2022-02-02 12:28:13 Re: [pgAdmin][RM-7110]: Cursor focus should be on first option for Maintenance and Backup/Restore