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-01-24 08:04:37
Message-ID: CAJ9T6SujBBD_wLwF0Npo2JKr2y6B_h-A-ks+Nur+gu6DQ5dO7Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

Attachment Content-Type Size
RM7016_v4.patch application/octet-stream 71.2 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2022-01-24 08:43:41 pgAdmin 4 commit: Port Dependent, dependencies, statistics panel to Rea
Previous Message Akshay Joshi 2022-01-24 06:34:42 pgAdmin 4 commit: Added validation for Hostname in the server dialog. F