Code Review of recent commits in related fork by SYudin July 22 2020

[tony-shannon]

Review of recent commits for PR here, from

https://github.com/yudin-s/PT-WC-on-Qewd-v1/commits/PTcollab

Key issues/questions/comments/feedback;

Components

https://github.com/yudin-s/PT-WC-on-Qewd-v1/tree/PTcollab/www/components

/pt-wq new components - assembly x 1, components x 7 , 1 external x 1
? is this a good idea, could should we be offering to refactor the core adminUI components?

I understand that ptwq root (replacing admin-root) is for
-handling routing and context management- but why do it this way?
I understand that ptwq-sidebar-nav-item.js is for
-handling role based access control to application headings etc - but it this solid enough

need view from Rob on this

/adminui-custom/components
admin crud custom
is this a good idea ? is being reused by events, vitals, patients, but do we need to specialise in a different way?
eg crud + charts, crud + calender, crud + map

Application pages

https://github.com/yudin-s/PT-WC-on-Qewd-v1/tree/PTcollab/www/pt-wc-q/js

/extended - for events, vitals, patients, reusing new admin-crud-custom,
these all require better documentation & consideration of refactoring as per above
eg crud + charts (vitals), crud + calender (events) etc

/utils - for utilities but we need better rationale for why these are needed
eg cSchemaLookUp, is this needed? to do what?
clientstorage & context manager- again why use this utility approach as opposed to others already present in wc-admin and qewd?

/pages subfolder - page seems empty? where is patient summary view handled?

other js files here need
-bit of clearout eg blankpage - should help with page loading
-consistent naming eg diagnosis page or diagnosis page? (remove state in names for now)
-fullCalendar List - seems a null file? or needs better documenting
-conferenceJS- need to consider how this is used in more detail- (ideally calendar/events set up conference meetings and can launch from there)

Styling & other coding issues

issue noted with use of app.js

root.sidebarTarget.classList.remove('sidebar-dark');
root.sidebarTarget.classList.add('d-none');

seems like wrong place to do it

[yudin-s]

@tony-shannon cSchemaLookUp is needed for find component by name BEFORE it will be rendered by mgWebComponents, for modify exists JS tree.

Example:

This utility function is used www/pt-wc-q/js/extended/patients.js at line 110 where I'm add button "Select patient" to the details card without editing CRUD deeply

[yudin-s]

@tony-shannon conferenceJS right now uses only for render component ptwq-jitsu.js . We can add more data inside this components and share/process them inside of the Jitsu loading.

Main issue of meet.jittsu are very similar to datatable - Jitsu NOT SUPORT server side rendering. We cannot render it inside virtual context, initialisation of the iframe must be happens on page where jitsu are placed and this why conference.js has too much code in hook

[yudin-s]

@tony-shannon /adminui-custom/components is not used anymore. It can be removed

[yudin-s]

@tony-shannon /pt-wq new components - assembly x 1, components x 7 , 1 external x 1

Let me explain:

1. We need to make new static area inside of admiui-root but we cannot edit this file so I copy them.
2. Other files has call to "adminui-root" but new one component with custom area named ptwq-root so lot functionality are broken. I have maked copy of this files and change only adminui-root to ptwq-root