Skip to content

1439 load state container markdown text blocks#1548

Closed
bannaarisamy-shanmugham-kanini wants to merge 20 commits intodevfrom
1439-load-state-container-markdown-text-blocks
Closed

1439 load state container markdown text blocks#1548
bannaarisamy-shanmugham-kanini wants to merge 20 commits intodevfrom
1439-load-state-container-markdown-text-blocks

Conversation

@bannaarisamy-shanmugham-kanini
Copy link
Copy Markdown
Contributor

Description

adding a load state setting on Container, Text and markdown blocks where it will have loading and Load Skeleton fields.
loading will feature a loading state of the blocks, and load skeleton will show selected skeleton, when the block is loading.

Changes Made

Added new Section named Load state under Container, Markdown and Text blocks block setting.

How to Test

  1. Drag in a Container/Text/Markdown block into workspace page.
  2. There will be an load state section in the block settings section.
  3. Choose the proper variables under loading field, and if needed add a skeleton, by default skeleton will be None.

Notes

@bannaarisamy-shanmugham-kanini bannaarisamy-shanmugham-kanini requested a review from a team as a code owner July 18, 2025 10:14
@bannaarisamy-shanmugham-kanini bannaarisamy-shanmugham-kanini linked an issue Jul 18, 2025 that may be closed by this pull request
2 tasks
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link
Copy Markdown

Title

1439 load state container markdown text blocks


User description

Description

adding a load state setting on Container, Text and markdown blocks where it will have loading and Load Skeleton fields.
loading will feature a loading state of the blocks, and load skeleton will show selected skeleton, when the block is loading.

Changes Made

Added new Section named Load state under Container, Markdown and Text blocks block setting.

How to Test

  1. Drag in a Container/Text/Markdown block into workspace page.
  2. There will be an load state section in the block settings section.
  3. Choose the proper variables under loading field, and if needed add a skeleton, by default skeleton will be None.

Notes


PR Type

Enhancement


Description

  • Introduce LoadingSkeleton SVG component

  • Extend blocks with loading and loadSkeleton props

  • Wrap block render in LoadingScreen loader

  • Add load state controls in block settings


Diagram Walkthrough

flowchart LR
  A["BlockData.loading & loadSkeleton"] --> B["Check isLoading"]
  B -- "true" --> C["Render LoadingSkeleton or Trigger"]
  B -- "false" --> D["Render actual block content"]
Loading

File Walkthrough

Relevant files

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Bundle Size

The inlined SVG in LoadingSkeleton.tsx is very large and could significantly increase the bundle size. Consider extracting or lazy loading the SVG or optimizing its paths to reduce output size.

export const LoadingSkeleton = (props) => {
    return (
        <div {...props}>
            <svg
                xmlns="http://www.w3.org/2000/svg"
                width="661"
                height="248"
                viewBox="0 0 661 248"
                fill="none"
            >
                <rect
                    width="661"
                    height="36"
                    transform="translate(0 16)"
                    fill="#F5F9FE"
                />
                <rect
                    x="16"
                    y="22"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint0_linear_18640_277229)"
                />
                <rect
                    x="180"
                    y="22"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint1_linear_18640_277229)"
                />
                <rect
                    x="345"
                    y="22"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint2_linear_18640_277229)"
                />
                <rect
                    x="509"
                    y="22"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint3_linear_18640_277229)"
                />
                <rect
                    width="661"
                    height="45"
                    transform="translate(0 52)"
                    fill="white"
                />
                <rect
                    x="16"
                    y="64"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint4_linear_18640_277229)"
                />
                <rect
                    x="180"
                    y="64"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint5_linear_18640_277229)"
                />
                <rect
                    x="344"
                    y="64"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint6_linear_18640_277229)"
                />
                <rect
                    x="509"
                    y="64"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint7_linear_18640_277229)"
                />
                <rect
                    width="661"
                    height="45"
                    transform="translate(0 97)"
                    fill="white"
                />
                <rect
                    x="16"
                    y="109"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint8_linear_18640_277229)"
                />
                <rect
                    x="180"
                    y="109"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint9_linear_18640_277229)"
                />
                <rect
                    x="345"
                    y="109"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint10_linear_18640_277229)"
                />
                <rect
                    x="509"
                    y="109"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint11_linear_18640_277229)"
                />
                <rect
                    width="661"
                    height="45"
                    transform="translate(0 142)"
                    fill="white"
                />
                <rect
                    x="16"
                    y="154"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint12_linear_18640_277229)"
                />
                <rect
                    x="180"
                    y="154"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint13_linear_18640_277229)"
                />
                <rect
                    x="344"
                    y="154"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint14_linear_18640_277229)"
                />
                <rect
                    x="509"
                    y="154"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint15_linear_18640_277229)"
                />
                <rect
                    width="661"
                    height="45"
                    transform="translate(0 187)"
                    fill="white"
                />
                <rect
                    x="16"
                    y="199"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint16_linear_18640_277229)"
                />
                <rect
                    x="180"
                    y="199"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint17_linear_18640_277229)"
                />
                <rect
                    x="345"
                    y="199"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint18_linear_18640_277229)"
                />
                <rect
                    x="509"
                    y="199"
                    width="100"
                    height="12"
                    rx="6"
                    fill="url(#paint19_linear_18640_277229)"
                />
                <defs>
                    <linearGradient
                        id="paint0_linear_18640_277229"
                        x1="116"
                        y1="28.1053"
                        x2="16"
                        y2="28.1053"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint1_linear_18640_277229"
                        x1="280"
                        y1="28.1053"
                        x2="180"
                        y2="28.1053"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint2_linear_18640_277229"
                        x1="445"
                        y1="28.1053"
                        x2="345"
                        y2="28.1053"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint3_linear_18640_277229"
                        x1="609"
                        y1="28.1053"
                        x2="509"
                        y2="28.1053"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint4_linear_18640_277229"
                        x1="116"
                        y1="70.1053"
                        x2="16"
                        y2="70.1053"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint5_linear_18640_277229"
                        x1="280"
                        y1="70.1053"
                        x2="180"
                        y2="70.1053"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint6_linear_18640_277229"
                        x1="444"
                        y1="70.1053"
                        x2="344"
                        y2="70.1053"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint7_linear_18640_277229"
                        x1="609"
                        y1="70.1053"
                        x2="509"
                        y2="70.1053"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint8_linear_18640_277229"
                        x1="116"
                        y1="115.105"
                        x2="16"
                        y2="115.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint9_linear_18640_277229"
                        x1="280"
                        y1="115.105"
                        x2="180"
                        y2="115.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint10_linear_18640_277229"
                        x1="445"
                        y1="115.105"
                        x2="345"
                        y2="115.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint11_linear_18640_277229"
                        x1="609"
                        y1="115.105"
                        x2="509"
                        y2="115.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint12_linear_18640_277229"
                        x1="116"
                        y1="160.105"
                        x2="16"
                        y2="160.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint13_linear_18640_277229"
                        x1="280"
                        y1="160.105"
                        x2="180"
                        y2="160.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint14_linear_18640_277229"
                        x1="444"
                        y1="160.105"
                        x2="344"
                        y2="160.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint15_linear_18640_277229"
                        x1="609"
                        y1="160.105"
                        x2="509"
                        y2="160.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint16_linear_18640_277229"
                        x1="116"
                        y1="205.105"
                        x2="16"
                        y2="205.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint17_linear_18640_277229"
                        x1="280"
                        y1="205.105"
                        x2="180"
                        y2="205.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint18_linear_18640_277229"
                        x1="445"
                        y1="205.105"
                        x2="345"
                        y2="205.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                    <linearGradient
                        id="paint19_linear_18640_277229"
                        x1="609"
                        y1="205.105"
                        x2="509"
                        y2="205.105"
                        gradientUnits="userSpaceOnUse"
                    >
                        <stop stop-color="#DBDBDB" stop-opacity="0.05" />
                        <stop offset="0.5" stop-color="#DBDBDB" />
                    </linearGradient>
                </defs>
            </svg>
        </div>
    );
};
Duplicated Logic

The loadTemplate function and loading-state rendering logic are duplicated in ContainerBlock and MarkdownBlock. Consider extracting it into a shared utility or higher-order component to adhere to DRY principles.

/**
 * Given a template string, loads the correct template to render in its place.
 * If the template doesn't exist, returns nothing.
 * @param {string} template The name of the template to load.
 * @returns {ReactElement} The loaded template, or nothing if it doesn't exist.
 */
const loadTemplate = (template: string): ReactElement => {
    if (template === "LoadingSkeleton") {
        return <LoadingSkeleton />;
    }
    return <LoadingScreen.Trigger />;
};
Nested LoadingScreen

The getLoadingChildren function wraps LoadingSkeleton with two nested LoadingScreen components (one in the helper and one in the render). Verify this nesting is intentional and simplify to a single LoadingScreen wrapper if possible.

const getLoadingChildren = () => {
    if (data?.loadSkeleton && data?.loadSkeleton === "LoadingSkeleton") {
        return (
            <LoadingScreen relative>
                <LoadingSkeleton />
            </LoadingScreen>
        );
    }
    return (
        <LoadingScreen relative>
            <LoadingScreen.Trigger />
        </LoadingScreen>
    );
};
// If the block is loading, return the loading children
// Otherwise, render the block with the text content
if (isLoading) return getLoadingChildren();

@QodoAI-Agent
Copy link
Copy Markdown

QodoAI-Agent commented Jul 18, 2025

PR Code Suggestions ✨

Latest suggestions up to f0214a9

CategorySuggestion                                                                                                                                    Impact
Possible issue
Preserve block wrapper during loading

Wrap the loading state return in the same container element to preserve style and
attributes. This ensures the block's styling (data.style) and HTML attributes
(...attrs) are applied even during loading.

libs/renderer/src/components/block-defaults/text-block/TextBlock.tsx [67-68]

-if (isLoading)
-    return <LoadingScreen relative>{getLoadingChildren()}</LoadingScreen>;
+if (isLoading) {
+    return (
+        <div style={data.style} {...attrs}>
+            <LoadingScreen relative>
+                {getLoadingChildren()}
+            </LoadingScreen>
+        </div>
+    );
+}
Suggestion importance[1-10]: 7

__

Why: Wrapping the loading state in the original <div> preserves the applied styles (data.style) and attributes (...attrs) during loading, maintaining layout consistency.

Medium
General
Skip placeholder for 'none' option

Explicitly handle a "none" skeleton option to avoid unintentionally rendering a
LoadingScreen.Trigger. Returning null for "none" prevents any placeholder from
showing.

libs/renderer/src/components/block-defaults/container-block/ContainerBlock.tsx [85-90]

-const getLoadingChildren = (template: string): ReactElement => {
+const getLoadingChildren = (template: string): ReactElement | null => {
     if (template === "LoadingSkeleton") {
         return <LoadingSkeleton />;
+    }
+    if (template === "none") {
+        return null;
     }
     return <LoadingScreen.Trigger />;
 };
Suggestion importance[1-10]: 5

__

Why: Adding a "none" case prevents rendering an unintended LoadingScreen.Trigger, which aligns with the default "none" skeleton setting without side effects.

Low
Simplify loading flag check

Simplify the loading check to directly compare against boolean true or string
"true". This avoids unnecessary property checks and string conversions.

libs/renderer/src/components/block-defaults/container-block/ContainerBlock.tsx [92-94]

-const isLoading =
-    data.hasOwnProperty("loading") &&
-    data.loading?.toString().toLowerCase() === "true";
+const isLoading = data.loading === true || data.loading === "true";
Suggestion importance[1-10]: 4

__

Why: Directly comparing data.loading to true or "true" removes the redundant property check and string methods, improving readability with minimal impact.

Low

Previous suggestions

Suggestions up to commit a124d23
CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle 'none' skeleton option

Add an explicit check for the "none" option so that when loadSkeleton is set to
"none" you return null instead of the trigger, respecting the user selection. Also
update the return type to include null.

libs/renderer/src/components/block-defaults/container-block/ContainerBlock.tsx [85-90]

-const loadTemplate = (template: string): ReactElement => {
+const loadTemplate = (template: string): ReactElement | null => {
     if (template === "LoadingSkeleton") {
         return <LoadingSkeleton />;
+    }
+    if (template === "none") {
+        return null;
     }
     return <LoadingScreen.Trigger />;
 };
Suggestion importance[1-10]: 7

__

Why: Adding an explicit "none" check respects the user selection and prevents unintentionally rendering the trigger when no skeleton is desired.

Medium
General
Memoize skeleton component

Wrap the LoadingSkeleton component in React.memo to prevent unnecessary re-renders
of the large SVG when its props don’t change. Add an explicit React import if needed
for memoization.

libs/renderer/src/assets/skeleton/LoadingSkeleton.tsx [1-2]

-export const LoadingSkeleton = (props) => {
+import React from "react";
+
+export const LoadingSkeleton = React.memo((props) => {
     return (
+        <div {...props}>
+            <svg
+                xmlns="http://www.w3.org/2000/svg"
+                width="661"
+                height="248"
+                viewBox="0 0 661 248"
+                fill="none"
+            >
+            ...
+            </svg>
+        </div>
+    );
+});
Suggestion importance[1-10]: 6

__

Why: Wrapping LoadingSkeleton in React.memo reduces unnecessary re-renders of the large SVG and improves performance with minimal changes.

Low
Unify loading wrapper

Instead of an early return, wrap both loading and content cases in a single
container to match the pattern in other blocks and avoid
dropping parent wrappers. This ensures consistent layout and styling.

libs/renderer/src/components/block-defaults/text-block/TextBlock.tsx [73-75]

-// If the block is loading, return the loading children
-// Otherwise, render the block with the text content
-if (isLoading) return getLoadingChildren();
+return (
+  <LoadingScreen relative>
+    {isLoading
+      ? (data.loadSkeleton === "LoadingSkeleton"
+          ? <LoadingSkeleton />
+          : <LoadingScreen.Trigger />)
+      : React.createElement(data.variant || "p", { style: data.style, ...attrs }, displayTxt)
+    }
+  </LoadingScreen>
+);
Suggestion importance[1-10]: 6

__

Why: Wrapping both loading and content cases in <LoadingScreen relative> ensures consistent layout and styling across blocks without altering core functionality.

Low
DRY up loadTemplate logic

Extract this loadTemplate helper into a shared utility or hook (e.g.
useLoadTemplate) to remove duplication between ContainerBlock and MarkdownBlock.
This will simplify maintenance and keep template logic in one place.

libs/renderer/src/components/block-defaults/markdown-block/MarkdownBlock.tsx [55-60]

-const loadTemplate = (template: string): ReactElement => {
-    if (template === "LoadingSkeleton") {
-        return <LoadingSkeleton />;
-    }
+// in libs/renderer/src/hooks/useLoadTemplate.ts
+import { ReactElement } from "react";
+import { LoadingScreen } from "@semoss/ui";
+import { LoadingSkeleton } from "../assets/skeleton/LoadingSkeleton";
+
+export function useLoadTemplate(template: string): ReactElement | null {
+    if (template === "LoadingSkeleton") return <LoadingSkeleton />;
+    if (template === "none") return null;
     return <LoadingScreen.Trigger />;
-};
+}
 
+// in MarkdownBlock.tsx
+import { useLoadTemplate } from "../../../hooks/useLoadTemplate";
+...
+<LoadingScreen relative>
+  {isLoading
+    ? useLoadTemplate(data.loadSkeleton)
+    : <ReactMarkdown remarkPlugins={[remarkGfm]}>{displayTxt}</ReactMarkdown>}
+</LoadingScreen>
+
Suggestion importance[1-10]: 5

__

Why: Extracting loadTemplate to a shared hook reduces duplication, but it introduces out-of-scope refactoring and extra logic not strictly required in this PR.

Low

@johbaxter johbaxter self-requested a review July 18, 2025 19:35
@johbaxter johbaxter self-assigned this Jul 18, 2025
@johbaxter
Copy link
Copy Markdown
Contributor

@bannaarisamy-shanmugham-kanini please fix the merge conflicts.

I would like this to get in tomorrow.

@bannaarisamy-shanmugham-kanini bannaarisamy-shanmugham-kanini marked this pull request as draft August 8, 2025 05:47
@bannaarisamy-shanmugham-kanini
Copy link
Copy Markdown
Contributor Author

Hi @Abhisheksaini129QA , I have committed all my changes and please validate my PR and let me know if any further changes are needed.

@Abhisheksaini129QA
Copy link
Copy Markdown

Hi @bannaarisamy-shanmugham-kanini , i have tested the above task , here is my observation

  • created 2 container in app- 2nd container is nested with markdown and 1st container contains only text
    created 3 query for container 2nd - python code with wait 5 second(with none loading Skelton) & its nested markdown with wait 10 second (with skelton loading Skelton)
    and for text its 15 seconds(with skelton loading Skelton)
    this scenario is working fine under ASYNC action but not in sync action type
    python code -
    import time
    print(True)
    time.sleep(10) # Pauses for 3 seconds
    print(False)

    could you please confirm this testing process
    CC- @bannaarisamy-shanmugham-kanini @rameshpaulraj
    FYI - video attached in the task ticket for better reference

@bannaarisamy-shanmugham-kanini bannaarisamy-shanmugham-kanini marked this pull request as ready for review August 19, 2025 14:14
Copy link
Copy Markdown
Contributor

@johbaxter johbaxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im taking this on and will use some of your code.

Where is the migration manager to add this property to all blocks

@johbaxter
Copy link
Copy Markdown
Contributor

Closed with: #1745

@johbaxter johbaxter closed this Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Container Load State

4 participants