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

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"