Conversation
❌ Deploy Preview for datalayer-core failed.
|
There was a problem hiding this comment.
Pull request overview
This PR expands “agent spaces” support and related UI capabilities by improving agent runtime cache behavior, adding optional route overrides in token/datasource views, introducing Google OAuth provider support, and renaming “Screenshot” functionality to “Screencapture” across the codebase.
Changes:
- Add configurable navigation routes for IAM tokens and datasources views.
- Introduce Google OAuth provider support (specs + login button + profile fetch hook) and improve agent runtime deletion cache invalidation.
- Rename Screenshot → Screencapture (hooks/components/utils/styles) and add system color-mode auto detection in the theme provider.
Reviewed changes
Copilot reviewed 104 out of 111 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| style/screencapture/index.css | New global screencapture overlay/crosshair styles. |
| src/views/iam-tokens/IAMTokens.tsx | Adds route override props for new/edit token navigation. |
| src/views/iam-tokens/IAMTokenNew.tsx | Adds route override prop for “List my Tokens” navigation. |
| src/views/datasources/Datasources.tsx | Adds route override props for new/edit datasource navigation. |
| src/views/datasources/DatasourceNew.tsx | Adds route override props for post-create and secrets navigation. |
| src/utils/index.ts | Updates (commented) export name to Screencapture. |
| src/utils/Screencapture.ts | Renames exported util to takeHTMLNodeScreencapture. |
| src/theme/useSystemColorMode.ts | New hook to follow OS prefers-color-scheme. |
| src/theme/index.ts | Re-exports useSystemColorMode. |
| src/theme/DatalayerThemeProvider.tsx | Adds auto colorMode resolution + optional theme/style overrides. |
| src/stories/Configure.mdx | Updates alt text wording (Screenshot → Screencapture). |
| src/state/substates/LayoutState.ts | Renames screenshot display type + actions to screencapture naming. |
| src/models/index.ts | Doc wording: “SDK” → “Client”. |
| src/models/tests/Space.test.ts | Renames mockSDK → mockClient. |
| src/models/tests/Snapshot.test.ts | Renames mockSDK → mockClient. |
| src/models/tests/RuntimeSnapshot.test.ts | Renames mockSDK → mockClient. |
| src/models/tests/Runtime.test.ts | Renames mockSDK → mockClient. |
| src/models/tests/Notebook.test.ts | Renames mockSDK → mockClient. |
| src/models/tests/Lexical.test.ts | Renames mockSDK → mockClient. |
| src/models/UserDTO.ts | Doc wording: “SDK” → “Client”. |
| src/models/SpaceDTO.ts | Doc wording: “SDK” → “Client”. |
| src/models/Secret.ts | Doc wording: “SDK” → “Client”. |
| src/models/RuntimeSnapshotDTO.ts | Doc wording: “SDK” → “Client”. |
| src/models/RuntimeDTO.ts | Doc wording: “SDK” → “Client”. |
| src/models/PageTag.tsx | Adds new page tags (agent/ai-agent/notebook). |
| src/models/Page.ts | Prefer CDN screencapture URL when present. |
| src/models/NotebookDTO.ts | Doc wording: “SDK” → “Client”. |
| src/models/LexicalDTO.ts | Doc wording: “SDK” → “Client”. |
| src/models/ItemDTO.ts | Doc wording: “SDK” → “Client”. |
| src/models/IAMProvidersSpecs.ts | Adds Google IAM provider spec. |
| src/models/IAMProviderUsers.ts | Adds Google user profile type. |
| src/models/HealthCheck.ts | Doc wording: “SDK” → “Client”. |
| src/models/EnvironmentDTO.ts | Doc wording: “SDK” → “Client”. |
| src/models/Datasource.ts | Doc wording: “SDK” → “Client”. |
| src/models/CreditsDTO.ts | Doc wording: “SDK” → “Client”. |
| src/index.ts | Renames exported handler type to ClientHandlers. |
| src/hooks/useScreenshot.tsx | Removes old Screenshot provider/hook. |
| src/hooks/useScreencapture.tsx | Adds new Screencapture provider/hook. |
| src/hooks/useCellOutputshot.ts | Switches to takeHTMLNodeScreencapture. |
| src/hooks/useCache.ts | Adds enableCodemode, improves delete invalidation, adds Google profile hook, maps search items. |
| src/hooks/layouts/index.ts | Exports LayoutScreencapture instead of LayoutScreenshot. |
| src/hooks/layouts/LayoutScreenshot.tsx | Removes old screenshot layout component. |
| src/hooks/layouts/LayoutScreencapture.tsx | New layout for screencapture flow + imports global styles. |
| src/hooks/layouts/LayoutScreencapture.css | Updates crosshair colors (now appears redundant with global CSS). |
| src/hooks/index.ts | Re-exports useScreencapture instead of useScreenshot. |
| src/examples/ReactRouterExample.tsx | Doc wording: “SDK hooks” → “Client hooks”. |
| src/components/screencapture/index.ts | Renames exports to Screencapture component/button. |
| src/components/screencapture/ScreencaptureButton.tsx | Renames + wires to layout store screencapture actions. |
| src/components/screencapture/Screencapture.tsx | Renames ScreenCapture component class to Screencapture. |
| src/components/progress/CreditsIndicator.tsx | Adds navigateTo option for click navigation. |
| src/components/progress/ConsumptionBar.tsx | Moves click/key handling to the wrapping button for accessibility. |
| src/components/auth/Login.tsx | Adds Google login button and provider init + inline icon. |
| src/client/utils/spacerUtils.ts | Doc wording: “SDK” → “Client”. |
| src/client/mixins/IAMMixin.ts | Doc wording: “SDK” → “Client”. |
| src/client/index.ts | Renames exported handler type to ClientHandlers. |
| src/client/constants.ts | Doc wording: “SDK” → “Client”. |
| src/client/base.ts | Renames SDKHandlers → ClientHandlers and related docs. |
| src/client/auth/types.ts | Doc wording: “SDK” → “Client”. |
| src/client/auth/index.ts | Doc wording: “SDK” → “Client”. |
| src/client/auth/AuthenticationManager.ts | Doc wording: “SDK” → “Client”. |
| src/client/tests/sdk.spacer.integration.test.ts | Renames integration test naming “SDK” → “Client”. |
| src/client/tests/sdk.runtimes.integration.test.ts | Renames integration test naming “SDK” → “Client”. |
| src/client/tests/sdk.models.integration.test.ts | Renames integration test naming “SDK” → “Client”. |
| src/client/tests/sdk.iam.integration.test.ts | Renames integration test naming “SDK” → “Client”. |
| src/api/utils/validation.ts | Doc wording: “SDK contract” → “Client contract”. |
| src/api/index.ts | Doc wording updates (but import-path guidance needs correction). |
| src/README.md | Updates public docs to use useScreencapture (currently mismatched with actual hook API). |
| package.json | Bumps package version to 0.0.27. |
| examples/streamlit-sklearn/app/main.py | Doc wording: “SDK” → “Client”. |
| examples/streamlit-sklearn/README.md | Doc wording: “SDK” → “Client”. |
| examples/pytorch-workloads/main.py | Doc wording: “SDK” → “Client”. |
| examples/pytorch-workloads/README.md | Doc wording: “SDK” → “Client”. |
| examples/nextjs-notebook/src/app/welcome/page.tsx | UI wording: “SDK” → “Client”. |
| examples/nextjs-notebook/README.md | Doc wording: “SDK” → “Client”. |
| examples/fastapi-sklearn/README.md | Doc wording: “SDK” → “Client”. |
| examples/decorator/README.md | Doc wording: “SDK” → “Client”. |
| examples/README.md | Doc wording: “SDK” → “Client”. |
| docs/docs/typescript/index.mdx | Doc wording: “SDK” → “Client”. |
| docs/docs/python/index.mdx | Doc wording: “SDK” → “Client”. |
| docs/docs/python/Tokens/index.mdx | Doc wording: “SDK auth” → “Client auth”. |
| dev/DEVELOPMENT.md | Updates docs wording but project structure path is incorrect. |
| datalayer_core/tests/test_decorators.py | Docstring wording: “SDK” → “Client”. |
| datalayer_core/client/init.py | Docstring wording: “SDK” → “Client”. |
| datalayer_core/cli/commands/authn.py | Docstrings/comments wording: “SDK” → “Client”. |
| README.md | Doc wording: “SDK” → “Client”. |
| CLAUDE.md | Doc wording changes but includes incorrect paths/claims. |
| API.md | Doc wording changes, but contains unresolved merge conflict markers. |
Comments suppressed due to low confidence (3)
API.md:613
- API.md contains unresolved Git merge conflict markers (e.g.,
<<<<<<< HEAD,=======,>>>>>>> main). This will break markdown rendering and is a sign the file wasn’t properly merged—please resolve the conflict and keep only the intended example snippet.
<<<<<<< HEAD
const sdk = new DatalayerClient({
token: process.env.DATALAYER_API_KEY,
=======
const client = new DatalayerClient({
token: process.env.DATALAYER_TOKEN,
>>>>>>> main
src/hooks/layouts/LayoutScreencapture.css:35
- This CSS file appears to be unused now that
LayoutScreencapture.tsximportsstyle/screencapture/index.cssinstead. Having two near-identical screencapture stylesheets is easy to drift out of sync—please either delete this file or switch the component import to use it consistently.
src/components/screencapture/Screencapture.tsx:34 - Component state property 'endX' is written, but it is never read.
Component state property 'endY' is written, but it is never read.
Component state property 'imageURL' is written, but it is never read.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** Called after successful Client method execution */ | ||
| afterCall?: (methodName: string, result: any) => void | Promise<void>; | ||
| /** Called when an SDK method throws an error */ | ||
| /** Called when an Client method throws an error */ | ||
| onError?: (methodName: string, error: any) => void | Promise<void>; |
There was a problem hiding this comment.
Grammar: “Called when an Client method throws an error” → “Called when a Client method throws an error”.
| <Link | ||
| href="javascript: return false;" | ||
| onClick={e => navigate('/settings/iam/secrets', e)} | ||
| onClick={e => navigate(secretsRoute, e)} | ||
| > |
There was a problem hiding this comment.
Using href="javascript: return false;" is a security/accessibility footgun and can be blocked by CSP. Prefer rendering this as a real button/link (href to a real URL) and call preventDefault() in the click handler if you need client-side navigation only.
| <Link | ||
| href="javascript: return false;" | ||
| onClick={e => navigate('/settings/iam/secrets', e)} | ||
| onClick={e => navigate(secretsRoute, e)} | ||
| > |
There was a problem hiding this comment.
Using href="javascript: return false;" is a security/accessibility footgun and can be blocked by CSP. Prefer a real URL (or a button) and call preventDefault() in the click handler if needed.
| <Link | ||
| href="javascript: return false;" | ||
| onClick={e => navigate('/settings/iam/secrets', e)} | ||
| onClick={e => navigate(secretsRoute, e)} | ||
| > |
There was a problem hiding this comment.
Using href="javascript: return false;" is a security/accessibility footgun and can be blocked by CSP. Prefer a real URL (or a button) and call preventDefault() in the click handler if needed.
| <Link | ||
| href="javascript: return false;" | ||
| onClick={e => navigate('/settings/iam/secrets', e)} | ||
| onClick={e => navigate(secretsRoute, e)} | ||
| > |
There was a problem hiding this comment.
Using href="javascript: return false;" is a security/accessibility footgun and can be blocked by CSP. Prefer a real URL (or a button) and call preventDefault() in the click handler if needed.
| * | ||
| * This module contains the low-level API functionality. For high-level object-oriented | ||
| * SDK classes, use @datalayer/core/sdk instead. | ||
| * Client classes, use @datalayer/core/sdk instead. | ||
| * |
There was a problem hiding this comment.
This comment says “use @datalayer/core/sdk instead”, but the high-level client lives under src/client and is exported from @datalayer/core/@datalayer/core/client (there is no sdk entrypoint in this repo). Please update the guidance to the correct import path.
| ```tsx | ||
| import { | ||
| useToast, | ||
| useBackdrop, | ||
| useScreenshot, | ||
| useScreencapture, | ||
| useUpload, | ||
| useVisibilityObserver, | ||
| } from '@datalayer/core'; | ||
|
|
||
| function InteractiveComponent() { | ||
| const { showToast } = useToast(); | ||
| const { showBackdrop, hideBackdrop } = useBackdrop(); | ||
| const { takeScreenshot } = useScreenshot(); | ||
| const { takeScreencapture } = useScreencapture(); | ||
| const { uploadFiles, uploading, progress } = useUpload(); |
There was a problem hiding this comment.
The example destructures takeScreencapture from useScreencapture(), but useScreencapture currently returns { closeScreencapture, displayScreencapture } (no takeScreencapture). Please update the example to use the actual API (e.g., the ScreencaptureButton/component or a util like takeHTMLNodeScreencapture).
| export type IScreencaptureProviderProps = { | ||
| children?: ReactNode; | ||
| zIndex?: number; | ||
| disableDarken?: boolean; | ||
| screenshotSurface?: (qfds: any) => JSX.Element; | ||
| }; | ||
|
|
||
| export function ScreencaptureProvider({ | ||
| children = null, | ||
| zIndex = 9999, | ||
| disableDarken = false, | ||
| screenshotSurface = undefined, | ||
| }: IScreencaptureProviderProps) { | ||
| const defaultScreencaptureSurface = { | ||
| position: 'fixed', | ||
| top: 0, | ||
| left: 0, | ||
| width: '100%', | ||
| height: '100%', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| backgroundColor: disableDarken ? 'initial' : 'rgba(0, 0, 0, 0.5)', | ||
| zIndex, | ||
| }; | ||
| const [screenshot, setScreencapture] = useState({ | ||
| open: false, | ||
| render: (closeScreencapture: any) => <></>, | ||
| }); |
There was a problem hiding this comment.
In this provider, several identifiers still use the old “screenshot” naming (screenshotSurface, screenshot state) even though the API is now “screencapture”. Renaming these to screencaptureSurface and screencapture (or similar) would avoid confusion and reduce the chance of mixing the old/new concepts.
| if (!iamProvidersAuthorizationURL[IAMProvidersSpecs.GitHub.name]) { | ||
| initIAMProvider(IAMProvidersSpecs.GitHub); | ||
| } | ||
| if (!iamProvidersAuthorizationURL[IAMProvidersSpecs.Google.name]) { | ||
| initIAMProvider(IAMProvidersSpecs.Google); | ||
| } |
There was a problem hiding this comment.
showGoogleLogin controls whether the Google login button is rendered, but the effect still calls initIAMProvider(IAMProvidersSpecs.Google) even when showGoogleLogin is false. Consider gating the Google initialization on showGoogleLogin to avoid unnecessary network calls/state updates.
| /** Inline Google "G" icon for the login button. */ | ||
| const GoogleIcon = () => ( | ||
| <svg width="16" height="16" viewBox="0 0 48 48"> | ||
| <path | ||
| fill="#EA4335" | ||
| d="M24 9.5c3.54 0 6.71 1.22 9.21 3.6l6.85-6.85C35.9 2.38 30.47 0 24 0 14.62 0 6.51 5.38 2.56 13.22l7.98 6.19C12.43 13.72 17.74 9.5 24 9.5z" | ||
| /> | ||
| <path | ||
| fill="#4285F4" | ||
| d="M46.98 24.55c0-1.57-.15-3.09-.38-4.55H24v9.02h12.94c-.58 2.96-2.26 5.48-4.78 7.18l7.73 6c4.51-4.18 7.09-10.36 7.09-17.65z" | ||
| /> | ||
| <path | ||
| fill="#FBBC05" | ||
| d="M10.53 28.59a14.5 14.5 0 0 1 0-9.18l-7.98-6.19a24.1 24.1 0 0 0 0 21.56l7.98-6.19z" | ||
| /> | ||
| <path | ||
| fill="#34A853" | ||
| d="M24 48c6.48 0 11.93-2.13 15.89-5.81l-7.73-6c-2.15 1.45-4.92 2.3-8.16 2.3-6.26 0-11.57-4.22-13.47-9.91l-7.98 6.19C6.51 42.62 14.62 48 24 48z" | ||
| /> | ||
| </svg> | ||
| ); |
There was a problem hiding this comment.
The inline SVG used as the Google button icon should be marked decorative to avoid noisy screen reader output (e.g., add aria-hidden="true" and focusable="false", or provide an accessible name/title if it’s meant to be announced).
No description provided.