Re: [pgAdmin][RM7016]: Port Dependent, dependencies, statistics panel to React. - Mailing list pgadmin-hackers

On Thu, Jan 20, 2022 at 10:30 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Adding to the previous mail:

1. Header and rows do not match when there is a scrollbar.

2. Empty box shown.

3. Please consider https://redmine.postgresql.org/issues/4852 as well.

On Thu, Jan 20, 2022 at 10:14 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.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.parkale@enterprisedb.com> wrote:

Hi Aditya,

Please find the updated patch.

On Wed, Jan 5, 2022 at 1:59 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.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.

On Wed, Jan 5, 2022 at 9:30 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:

Hi Aditya

Can you please review it?

On Wed, 5 Jan, 2022, 9:18 am Pradip Parkale, <pradip.parkale@enterprisedb.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

"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

"Don't Complain about Heat, Plant a TREE"

--

Thanks,

Aditya Toshniwal

pgAdmin Hacker | Software Architect | edbpostgres.com

"Don't Complain about Heat, Plant a TREE"

--

Thanks & Regards,

Pradip Parkale
Software Engineer | EnterpriseDB Corporation