chore: init theme design#20563
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ebfd6728c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f84a9dca2d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fe151d6ef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| The SQL Editor therefore maintains **one active Monaco theme**, derived from the | ||
| **foreground code surface** and applied by a single controller (driven by the current | ||
| tab `mode` + selected `themeId`) that calls `monaco.editor.setTheme(\`bb-${active.id}\`)` | ||
| whenever either changes (today set once at `MonacoEditor.tsx:346`): |
There was a problem hiding this comment.
In runtime modes where the VSCode theme service override prevents custom themes from registering, driving the controller with raw bb-${active.id} can leave Monaco stuck on whatever global theme was last applied. I checked the current frontend/src/react/components/monaco/core.ts: getResolvedTheme() exists specifically because calling setTheme with an unregistered custom theme is a silent no-op, and initializeTheme() only adds names to registeredThemes after defineTheme succeeds. The design should keep that registration/fallback path for every generated preset before centralizing setTheme, otherwise named themes can fail in those existing runtime modes.
Useful? React with 👍 / 👎.
| - `SQLEditorHomePage` portals the mobile FAB and the sidebar overlay to | ||
| `getLayerRoot("overlay")` (`SQLEditorHomePage.tsx:160-182, 191-208`). | ||
| - `AccessGrantRequestDrawer` (rendered by `RequestDrawerHost`, `RequestDrawerHost.tsx:74`) | ||
| is a `Sheet` that portals to the overlay layer **and** hosts its own `MonacoEditor` | ||
| (`AccessGrantRequestDrawer.tsx:262`). |
There was a problem hiding this comment.
This portal inventory is still incomplete: a repo-wide search for createPortal( / getLayerRoot("overlay") under frontend/src/react/components/sql-editor also finds themed overlay surfaces such as ConnectChooser's dropdown (bg-background, border-control-border), schema/database hover panels, EllipsisCell tooltips, and the RequestRoleSheet path hosted by RequestDrawerHost. Under a dark/named SQL Editor theme these portals mount under the app-global overlay root and will keep global light CSS variables unless each is wrapped/re-scoped too, so the implementation plan and visual tests should enumerate and cover them rather than only the mobile FAB/sidebar and access-grant drawer.
Useful? React with 👍 / 👎.
1 participant
No description provided.