feat : Query manager separated to different tabs

[stepinfwd]

closes #9438

[johnsoncherian]

/review

[TooljetBot]

Code Review Agent Run #46c012

• AI Based Review: ✔️ Successful

Code Review Overview

• Summary: The changes primarily focus on updating styles in SCSS files, enhancing QueryManager components, improving UI/UX, and adding new properties and logic for better functionality. Significant updates include the addition of tabs in QueryManagerHeader, restructuring of the Transformation component, and various styling adjustments for better layout and user experience.
• Files: 40
• Issue found: Total - 10, High importance - 10      See detailed feedback ->
• Code change type: Refactoring, UI/UX Improvement, Performance Improvement
• Unit tests added: False
• Estimated effort to review (1-5, lower is better): 3

High-level Feedback

Consider moving inline styles to CSS classes for better maintainability, ensure parent containers have defined heights to avoid layout issues, and validate new props to prevent unexpected behavior. Additionally, optimize useEffect dependencies to ensure consistent behavior and remove redundant props to simplify component logic.

Detailed Feedback

📄 frontend/src/Editor/QueryManager/Components/QueryManagerBody.jsx

Issues: Total- 1, High importance- 1

Line 342-342

🔴 High importance - 1

📄 frontend/src/_styles/colors.scss

Issues: Total- 2, High importance- 2

Line 43-43	🔴 High importance - 1
Line 223-223	🔴 High importance - 1

📄 frontend/src/_styles/queryManager.scss

Issues: Total- 1, High importance- 1

Line 308-308

🔴 High importance - 1

📄 frontend/src/Editor/CodeEditor/MultiLineCodeEditor.jsx

Issues: Total- 1, High importance- 1

Line 257-257

🔴 High importance - 1

📄 frontend/src/Editor/QueryManager/Components/QueryManagerHeader.jsx

Issues: Total- 1, High importance- 1

Line 94-102

🔴 High importance - 1

📄 frontend/src/_helpers/appUtils.js

Issues: Total- 1, High importance- 1

Line 1144-1148

🔴 High importance - 1

📄 frontend/src/_ui/Icon/solidIcons/ArrowDownTriangle.jsx

Issues: Total- 1, High importance- 1

Line 3-3

🔴 High importance - 1

📄 frontend/src/Editor/QueryManager/Components/Preview.jsx

Issues: Total- 1, High importance- 1

Line 33-33

🔴 High importance - 1

📄 frontend/src/Editor/QueryManager/QueryManager.jsx

Issues: Total- 1, High importance- 1

Line 28-32

🔴 High importance - 1

AI Code Review powered by

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: Inline styles are used for setting the height and overflow properties. This can lead to difficulties in maintaining and updating the styles, especially if these styles need to be reused or modified in multiple places.

Fix: Move the inline styles to a CSS class to ensure better maintainability and consistency across the codebase.

Code suggestion

 @@ -341,7 +341,7 @@
       className={'query-details ${selectedDataSource?.kind === 'tooljetdb' ? 'tooljetdb-query-details' : ''}'}
 -     style={{ height: 'calc(100% - ${previewHeight + 40}px )', overflowY: 'auto' }} //40px in preview header height
 +     className={'query-details ${selectedDataSource?.kind === 'tooljetdb' ? 'tooljetdb-query-details' : ''} custom-query-details'}
    >

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: There is an extraneous semicolon on line 43, which could cause issues in the CSS compilation or lead to unexpected behavior.

Fix: Remove the extraneous semicolon to ensure proper CSS syntax.

Code suggestion

 @@ -42,7 +42,6 @@
  $color-light-indigo-04: #E6EDFE;
 -;
  $color-dark-indigo-04: #1C274F;

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: Missing semicolon in the CSS rule for '.text-default'. This can cause issues in CSS parsing and lead to unexpected behavior in styles.

Fix: Add a semicolon at the end of the 'color' property in the '.text-default' rule.

Code suggestion

 @@ -222,7 +222,7 @@
  .text-default {
    color: var(--text-default);
  }

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: Setting the height to 100% for the '.query-definition-pane' class without considering its parent container's height can lead to layout issues, especially if the parent container does not have a defined height.

Fix: Ensure that the parent container of '.query-definition-pane' has a defined height or use a more flexible layout approach such as flexbox or grid to manage the height.

Code suggestion

 @@ -307,7 +307,7 @@
  .query-definition-pane {
 -    height: 100%;
 +    flex: 1;

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: The 'readOnly' and 'editable' props are both being used, which can cause confusion and potential conflicts in the component's behavior. Typically, 'readOnly' should suffice for controlling editability.

Fix: Remove the 'editable' prop and rely solely on the 'readOnly' prop to control the editability of the component.

Code suggestion

 @@ -42,8 +42,7 @@ const MultiLineCodeEditor = (props) => {
      showPreview,
      paramLabel = '',
      delayOnChange = true, // Added this prop to immediately update the onBlurUpdate callback
 -    readOnly = false,
 -    editable = true,
 +    readOnly = false,
    } = props;
const [currentValue, setCurrentValue] = React.useState(() => initialValue);

@@ -254,8 +253,7 @@ const MultiLineCodeEditor = (props) => {

overflowY: 'auto',

}}

className={'codehinter-multi-line-input'}

indentWithTab={true}


       readOnly={readOnly}



       editable={editable} //for transformations in query manager





       readOnly={readOnly}
     />
   </div>
   {showPreview && (

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: The 'tabs' array is defined within the component function, which means it will be recreated on every render. This can lead to unnecessary re-renders and performance issues, especially if the component re-renders frequently.

Fix: Move the 'tabs' array definition outside of the component function to ensure it is only created once.

Code suggestion

 @@ -93,6 +93,15 @@
  };
+const tabs = [

{ id: 1, label: 'Setup' },
{
id: 2,
label: 'Transformation',
condition: selectedQuery?.kind !== 'runpy' && selectedQuery?.kind !== 'runjs',
},
{ id: 3, label: 'Settings' },

+];

const QueryManagerHeader = forwardRef(({ darkMode, options, editorRef, selectedQuery, queries, runQuery, previewButtonOnClick, buttonLoadingState, isInDraft, executeQueryNameUpdation, queryName, isVersionReleased, showCreateQuery }, ref) => {

const renderRunButton = () => {

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: The new conditional block that sets the preview panel state ('setPreviewPanelExpanded', 'setPreviewLoading', 'setPreviewData') is executed before checking if the 'query' exists. This can lead to unnecessary state updates and potential performance issues if the query is not found.

Fix: Move the conditional block inside the 'if (query)' block to ensure state updates only occur when the query exists.

Code suggestion

 @@ -1143,11 +1143,6 @@

if (shouldSetPreviewData) {
setPreviewPanelExpanded(true);
setPreviewLoading(true);
queryPreviewData && setPreviewData('');
}

if (query) {

dataQuery = JSON.parse(JSON.stringify(query));


if (shouldSetPreviewData) {

 setPreviewPanelExpanded(true);



 setPreviewLoading(true);



 queryPreviewData && setPreviewData('');


}

} else {

toast.error('No query has been associated with the action.');

return;

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: The addition of the 'style' prop to the 'ArrowDownTriangle' component without validation or default values can lead to unexpected styling issues if incorrect styles are passed.

Fix: Add prop-types to validate the 'style' prop and provide default values to ensure consistent behavior.

Code suggestion

 @@ -1,13 +1,14 @@
 +import PropTypes from 'prop-types';
 const ArrowDownTriangle = ({ fill = '#C1C8CD', width = '24', className = '', viewBox = '0 0 24 24', style = {} }) => (
   <svg
     xmlns="http://www.w3.org/2000/svg"
     width={width}
     height={width}
     viewBox={viewBox}
     fill="none"
     className={className}
     style={style}
   >
     <path
       fill-rule="evenodd"
       clip-rule="evenodd"
       d="M9.20923 10.7525C9.00495 10.4589 8.94384 10.0173 9.05439 9.63373C9.16495 9.25012 9.4254 9 9.7143 9H18.2857C18.5746 9 18.835 9.25012 18.9456 9.63373C19.0562 10.0173 18.995 10.4589 18.7907 10.7525L14.7576 16.5491C14.3392 17.1503 13.6608 17.1503 13.2424 16.5491L9.20923 10.7525Z"
       fill={fill}
     />
   </svg>
 );
 +ArrowDownTriangle.propTypes = {
 +  fill: PropTypes.string,
 +  width: PropTypes.string,
 +  className: PropTypes.string,
 +  viewBox: PropTypes.string,
 +  style: PropTypes.object,
 +};
 export default ArrowDownTriangle;

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: The 'calculatePreviewHeight' function is called inside a 'useEffect' hook without any dependencies, which means it will only run once when the component mounts. This could lead to issues if the 'height' or 'previewPanelExpanded' values change after the initial render, as the height calculation will not be updated.

Fix: Add 'height' and 'previewPanelExpanded' as dependencies to the 'useEffect' hook to ensure the height calculation is updated whenever these values change.

Code suggestion

 @@ -33,7 +33,7 @@
   useEffect(() => {
     calculatePreviewHeight(height, previewPanelExpanded);
     // eslint-disable-next-line react-hooks/exhaustive-deps
 -  }, []);
 +  }, [height, previewPanelExpanded]);

[TooljetBot]

Bito Code Review Agent Run #46c012 - 06/14/2024, 11:19 am

🔴 High importance

Issue: The useEffect hook that sets the active tab based on the selected query kind is missing a dependency on 'selectedQuery?.kind'. This can lead to inconsistent behavior if 'selectedQuery' changes but its 'id' does not.

Fix: Add 'selectedQuery?.kind' to the dependency array of the useEffect hook to ensure it re-runs when the kind of the selected query changes.

Code suggestion

 @@ -32,7 +32,7 @@ const QueryManager = ({ mode, appId, darkMode, apps, allComponents, appDefinitio
   }, [selectedQuery?.id, selectedQuery?.kind]);