Conversation
maxelkins
temporarily deployed
to
previews/1385/merge
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new documentation plan describing current button tech debt in the editor UI and proposing a phased migration to the RPF Design System Button, with emphasis on theming and icon strategy.
Changes:
- Introduces
docs/ButtonRefactor.mdoutlining current button usage across the codebase and a proposed migration plan. - Documents proposed prop mappings, icon approach, and a theming strategy using CSS custom properties.
- Captures verification steps and success metrics for the migration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+232
to
+251
| :root { | ||
| // Editor-specific theming using existing RPF Button custom properties | ||
|
|
||
| // Note ideally one of these should be a default, | ||
| // meaning only overriding some variables for the other theme. | ||
|
|
||
| .--light { | ||
| // Light mode: different custom property values | ||
| --rpf-button-background-color: var(--rpf-teal-800); | ||
| --rpf-button-background-color-hover: var(--rpf-teal-400); | ||
| --rpf-button-text-color: var(--rpf-white); | ||
| } | ||
|
|
||
| .--dark { | ||
| // Dark mode: just override custom property values | ||
| --rpf-button-background-color: var(--rpf-grey-700); | ||
| --rpf-button-background-color-hover: var(--rpf-grey-600); | ||
| --rpf-button-text-color: var(--rpf-white); | ||
| // Icons inherit text color automatically | ||
| } |
Comment on lines
+57
to
+72
| | SaveButton | [src/components/SaveButton/SaveButton.jsx](src/components/SaveButton/SaveButton.jsx) | Save project functionality | | ||
| | DownloadButton | [src/components/DownloadButton/DownloadButton.jsx](src/components/DownloadButton/DownloadButton.jsx) | Download project functionality | | ||
| | ProjectName | [src/components/ProjectName/ProjectName.jsx](src/components/ProjectName/ProjectName.jsx) | Edit/save project name | | ||
| | ProjectBar | [src/components/ProjectBar/ProjectBar.jsx](src/components/ProjectBar/ProjectBar.jsx) | Scratch save button | | ||
| | FilePanel | [src/components/Menus/Sidebar/FilePanel/FilePanel.jsx](src/components/Menus/Sidebar/FilePanel/FilePanel.jsx) | File management actions | | ||
| | ProjectsPanel | [src/components/Menus/Sidebar/ProjectsPanel/ProjectsPanel.jsx](src/components/Menus/Sidebar/ProjectsPanel/ProjectsPanel.jsx) | Project listing actions | | ||
| | DownloadPanel | [src/components/Menus/Sidebar/DownloadPanel/DownloadPanel.jsx](src/components/Menus/Sidebar/DownloadPanel/DownloadPanel.jsx) | Download panel buttons | | ||
| | InstructionsPanel | [src/components/Menus/Sidebar/InstructionsPanel/InstructionsPanel.jsx](src/components/Menus/Sidebar/InstructionsPanel/InstructionsPanel.jsx) | Instruction navigation | | ||
|
|
||
| ### **Button (custom) usages (15 components)** | ||
|
|
||
| | Component | File Path | Usage Context | | ||
| | ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------- | | ||
| | RunButton | [src/components/RunButton/RunButton.jsx](src/components/RunButton/RunButton.jsx) | Run code execution | | ||
| | StopButton | [src/components/RunButton/StopButton.jsx](src/components/RunButton/StopButton.jsx) | Stop code execution | | ||
| | GeneralModal | [src/components/Modals/GeneralModal.jsx](src/components/Modals/GeneralModal.jsx) | Modal action buttons | |
Comment on lines
+75
to
+84
| | RenameFileModal | [src/components/Modals/RenameFileModal.jsx](src/components/Modals/RenameFileModal.jsx) | Rename modal buttons | | ||
| | Dropdown | [src/components/Menus/Dropdown/Dropdown.jsx](src/components/Menus/Dropdown/Dropdown.jsx) | Dropdown toggle button | | ||
| | SidebarBarOption | [src/components/Menus/Sidebar/SidebarBarOption.jsx](src/components/Menus/Sidebar/SidebarBarOption.jsx) | Sidebar navigation | | ||
| | SidebarBar | [src/components/Menus/Sidebar/SidebarBar.jsx](src/components/Menus/Sidebar/SidebarBar.jsx) | Sidebar container | | ||
| | ProgressBar | [src/components/Menus/Sidebar/InstructionsPanel/ProgressBar/ProgressBar.jsx](src/components/Menus/Sidebar/InstructionsPanel/ProgressBar/ProgressBar.jsx) | Step navigation | | ||
| | OutputViewToggle | [src/components/Editor/Runners/PythonRunner/OutputViewToggle.jsx](src/components/Editor/Runners/PythonRunner/OutputViewToggle.jsx) | Split/tabbed view toggle | | ||
| | NewInputPanelButton | [src/components/Editor/NewInputPanelButton/NewInputPanelButton.jsx](src/components/Editor/NewInputPanelButton/NewInputPanelButton.jsx) | Add editor panel | | ||
| | EditorInput | [src/components/Editor/EditorInput/EditorInput.jsx](src/components/Editor/EditorInput/EditorInput.jsx) | Tab close buttons | | ||
| | ToastCloseButton | [src/utils/ToastCloseButton.jsx](src/utils/ToastCloseButton.jsx) | Toast notification close | | ||
| | Notifications | [src/utils/Notifications.js](src/utils/Notifications.js) | Custom notification close | |
|
|
||
| **Example of usage**: | ||
|
|
||
| - [Dropdown.jsx](src/components/Menus/Dropdown/Dropdown.jsx) currently uses `buttonImage` prop, and multiple components use `ButtonIcon` React elements. |
| --rpf-button-border-width | ||
| --rpf-button-lg-height | ||
| --rpf-button-sm-height | ||
| --rpf-button-min-target-size |
danhalson
reviewed
Mar 16, 2026
|
|
||
| ## Success metrics | ||
|
|
||
| - All tests passing |
Contributor
There was a problem hiding this comment.
I think it would be beneficial to bolster test coverage first in select areas, at the very least some visual regression tests, which would give us more confidence in the most impacted areas that there are no side effects
Contributor
There was a problem hiding this comment.
Some of these issues are also affecting editor-standalone, so it would be good to make that clearer if possible
Contributor
|
We can add it to the project if it would be useful for others, but I think things like this are easier to edit and keep up to date as issue- otherwise any minor change to this file will need approval to do. Also worth noting that no issues are being automatically deleted, they just are marked as closed and can be re-opened if needed. |
Contributor
Author
Happy to make it an issue - yeah closing is what I meant 😅 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds detailed documentation around current button tech debt and discusses potential approach to tackling this debt.
Notes
If this is the wrong place for this, I can move this into an issue (worried it'll just go stale and get closed) or make an ADR in the dp docs.