Skip to content

ToolBar Buttons refactoring#1180

Open
AnnieeBennie wants to merge 3 commits into
masterfrom
Toolbar-Buttons-refactoring
Open

ToolBar Buttons refactoring#1180
AnnieeBennie wants to merge 3 commits into
masterfrom
Toolbar-Buttons-refactoring

Conversation

@AnnieeBennie

Copy link
Copy Markdown
Collaborator

separated Toggles into a new component
separated styles

@netlify

netlify Bot commented Jun 23, 2026

Copy link
Copy Markdown

Deploy Preview for voluble-nougat-015dd1 ready!

Name Link
🔨 Latest commit fd08fa5
🔍 Latest deploy log https://app.netlify.com/projects/voluble-nougat-015dd1/deploys/6a44edac48e2d80008d469ff
😎 Deploy Preview https://deploy-preview-1180--voluble-nougat-015dd1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@mircealungu mircealungu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same good direction as #1181. Two things:

  1. .sc.js = styled-components in this repo — every other .sc.js exports styled.…, but the new ones export plain JS objects, so the name is misleading. Either make them real styled-components or rename to plain styles.js. (Toggle/icon/text-size split itself is clean, and I confirmed the toggle()onToggle(!checked) change is behavior-equivalent. 👍)

  2. Dead duplicated exports in ToolbarButtons.sc.js — only 4 of the 10 exports are imported. toolbarButtonTrigger / toolbarIcon now live in SettingsIconButton.sc.js, and fontSizeButtonStyle / fontSizeRow / fontSizeControls / fontSizeValue now live in TextSizeControl.sc.js. The toolbar copies are never used — please delete those 6 so the styles aren't defined in two places.

@AnnieeBennie

Copy link
Copy Markdown
Collaborator Author

@mircealungu fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants