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 08:44:18
Message-ID: CANxoLDd3W0g=vrE4BuXwPTZWwGS45LuGJQLzKfzfXGfXUuyVqQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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*

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Nikhil Mohite 2022-01-25 06:15:23 [pgAdmin][RM-7123]: v6.3 restore generates incorrect options for schema
Previous Message Akshay Joshi 2022-01-24 08:43:41 pgAdmin 4 commit: Port Dependent, dependencies, statistics panel to Rea