Conversation
✅ Deploy Preview for datalayer-core ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Updates the Datalayer theming implementation to use a dedicated Datalayer Primer theme/palette and adjusts navigation + caching behaviors to better align with expected UX/data freshness.
Changes:
- Introduces a Datalayer accessible color system and expands Primer theme overrides for light/dark schemes.
- Updates
DatalayerThemeProviderto apply the Datalayer theme and set base/button styling defaults. - Enhances navigation (scroll-to-top + numeric history navigation) and adjusts cache behavior for secrets + query options.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/theme/DatalayerThemeProvider.tsx | Applies the Datalayer theme in ThemeProvider and sets light/dark base styles + primary button CSS variables. |
| src/theme/DatalayerTheme.ts | Adds datalayerColors, defines light/dark theme overrides, and exports the merged Primer+Datalayer theme. |
| src/hooks/useNavigate.tsx | Adds scroll-to-top on React Router navigations and supports numeric history navigation in the native navigator. |
| src/hooks/useCache.ts | Stops pre-populating secret detail cache from list results, extends useSecret query options, and changes OAuth2 link mutation return value. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const baseStyleDark: CSSProperties = { | ||
| backgroundColor: datalayerColors.black, | ||
| color: datalayerColors.white, | ||
| fontSize: 'var(--text-body-size-medium)', |
There was a problem hiding this comment.
BaseStyles is being forced to hard-coded hex backgroundColor/color values. This bypasses Primer theme CSS variables, so if colorMode is auto (or if dayScheme/nightScheme are customized) the computed scheme can diverge from these inline colors. Consider going back to using the Primer tokens (e.g., var(--bgColor-default) / var(--fgColor-default)) or deriving the resolved scheme from Primer and only overriding when you can reliably detect dark vs light.
| url: `${configuration.iamRunUrl}/api/iam/v1/oauth2/authz/url/link?${queryString}`, | ||
| }); | ||
| return resp.autorization_url; | ||
| return resp; |
There was a problem hiding this comment.
useOAuth2AuthorizationLinkURL is documented/typed as returning an OAuth2 authorization URL (autorization_url: string), but the mutation currently returns the entire resp object. This makes the hook’s return type inconsistent with useOAuth2AuthorizationURL and will break callers expecting a string URL. Return resp.autorization_url (or update the response type and docstring if the endpoint intentionally returns a different shape).
| return resp; | |
| return resp.autorization_url; |
Includes also minor fix for useCache.