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

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"