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

From: Pradip Parkale <pradip(dot)parkale(at)enterprisedb(dot)com>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin][RM7016]: Port Dependent, dependencies, statistics panel to React.
Date: 2022-01-23 07:31:09
Message-ID: CAJ9T6Ss7C98ACrdehW-uxqKa7B96gmg=GAMPUA1iDo8woJnL3w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

Attachment Content-Type Size
RM7016_v3.patch application/octet-stream 70.3 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2022-01-24 05:39:10 Re: [pgAdmin][RM7016]: Port Dependent, dependencies, statistics panel to React.
Previous Message Akshay Joshi 2022-01-21 14:11:44 Re: [pgAdmin]: Remove "Move objects to..." option from tablespaces sub-node