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

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Pradip Parkale <pradip(dot)parkale(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-01-24 05:39:10
Message-ID: CANxoLDe8Jh6jMmQOjcw8r4TNsPNZiSg6x7b8Cs2TRr1u2FgiCQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2022-01-24 06:34:42 pgAdmin 4 commit: Added validation for Hostname in the server dialog. F
Previous Message Pradip Parkale 2022-01-23 07:31:09 Re: [pgAdmin][RM7016]: Port Dependent, dependencies, statistics panel to React.