Skip to content

fix: 🐛 restore changes lost in PR 841-845 merge conflict resolution#878

Merged
punkbit merged 8 commits intomainfrom
fix/restore-changes-lost-in-PR-resolution-841-845-merge-conflict-resolution
Mar 6, 2026
Merged

fix: 🐛 restore changes lost in PR 841-845 merge conflict resolution#878
punkbit merged 8 commits intomainfrom
fix/restore-changes-lost-in-PR-resolution-841-845-merge-conflict-resolution

Conversation

@punkbit
Copy link
Collaborator

@punkbit punkbit commented Mar 6, 2026

Why?

Help correct bad merge conflict decisions/resolutions caused by squash-merging chained branches and loss of base context. Public API changes from PR #845 had leaked into PR #841, resulting in incorrect decisions when merging main back to the target branch.

💡 Relates to #879

How?

Preview?

N/A

punkbit added 5 commits March 6, 2026 14:56
Phase 1: Restore CrossButton, EmptyButton, GridCenter as standalone components

- Created src/components/CrossButton/ (moved from Common/CrossButton.tsx)
- Created src/components/EmptyButton/ (moved from Common/EmptyButton.tsx)
- Created src/components/GridCenter/ (moved from Common/GridCenter.tsx)
- Added index.ts exports for each new component
- Updated Common/index.ts to remove exports for moved components
- Updated imports in:
  - Dialog.tsx, Flyout.tsx (CrossButton)
  - Popover.tsx, CodeBlock.tsx, Collapsible.tsx (EmptyButton)
  - 4 story files (GridCenter)
Phase 3: Clean Up Common directory and reorganize imports

- FormContainer/index.ts: Added exports for FormRoot, Error, FormElementContainer
- Common/index.ts: Removed all component exports, keeping only type re-exports
- SplitButton.tsx: Updated to import BaseButton directly from @/components/Button/BaseButton
- Switch.tsx, RadioGroup.tsx, InternalSelect.tsx, InputWrapper.tsx, Checkbox.tsx, FormContainer.tsx:
  Changed imports from @/components/Common to @/components/FormContainer or relative imports

Removes Common/ as a component barrel, keeping it only for type re-exports.
Removes the entire Common/ directory including:
- BaseButton.tsx (duplicate of Button/BaseButton.tsx)
- Error.tsx (duplicate of FormContainer/Error.tsx)
- FormElementContainer.tsx (duplicate of FormContainer/FormElementContainer.tsx)
- FormRoot.tsx (duplicate of FormContainer/FormRoot.tsx)
- EllipsisContainer.tsx (unused, local versions exist in other components)
- index.ts (type re-exports no longer needed)

All imports have been updated to use the canonical locations in previous commits.
@changeset-bot
Copy link

changeset-bot bot commented Mar 6, 2026

🦋 Changeset detected

Latest commit: e91a340

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@clickhouse/click-ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Claude finished @punkbit's task in 1m 23s —— View job


PR Review

  • Read conventions
  • Analyze PR diff and changed files
  • Review component structure changes
  • Post final review

This PR cleanly restores the intended architecture: dissolving the Common/ catch-all directory and giving each utility component its own focused directory. The changes are well-scoped and surgical.

Verified:

  • ✅ All @/components/Common imports are gone — zero remaining references
  • EmptyButton, CrossButton, GridCenter each moved to their own directory with proper index.ts
  • FormRoot, Error, FormElementContainer consolidated into FormContainer/ and re-exported from its index.ts
  • ✅ The old EllipsisContainer from Common was not used externally (confirmed by search) — safe to delete
  • BaseButton was already present in Button/BaseButton.tsx — the Common copy was a duplicate, correctly removed

💡 Suggestion (non-blocking)

  • SplitButton/SplitButton.tsx:4 imports BaseButton via a direct file path @/components/Button/BaseButton, bypassing the module's index.ts. This is a pre-existing pattern (not introduced here), but it creates an implicit dependency on implementation details. A future improvement would be to export BaseButton from Button/index.ts so it's part of the module's public contract.

✅ Looks great — clean restoration of the right architecture. Nice work! 🙌

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Claude finished @punkbit's task in 1m 0s —— View job


PR Review

  • Read conventions
  • Analyze PR diff and changed files
  • Review component structure changes
  • Post final review

Clean restoration of the intended architecture — dissolving Common/ and giving each utility component its own focused directory. The changes are surgical and well-scoped.

Verified:

  • ✅ All @/components/Common imports replaced with correct module paths
  • EmptyButton, CrossButton, GridCenter each moved to individual directories with index.ts
  • FormRoot, Error, FormElementContainer collocated in FormContainer/ and re-exported from its index.ts
  • BaseButton duplicate in Common/ removed — SplitButton now imports from @/components/Button/BaseButton directly
  • src/index.ts unchanged — no unintended public API expansion

💡 Suggestion (non-blocking)

  • FormContainer/index.ts:3-5: FormRoot, Error, and FormElementContainer are now exported from the module's public barrel. These are internal styled primitives not exposed via src/index.ts, so there's no public API impact — but exporting Error in particular could cause confusion since it shadows the JS built-in. A future improvement: move them to a FormContainer/internal.ts file to make the private-use intent explicit.

  • SplitButton/SplitButton.tsx:4: pre-existing pattern (not introduced here) — imports BaseButton via @/components/Button/BaseButton bypassing Button/index.ts. Consider exporting BaseButton from Button/index.ts to make it part of the module's contract.


✅ Looks great — clean, minimal, and correct. Nice work! 🙌

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Claude finished @punkbit's task in 1m 4s —— View job


PR Review

  • Read conventions
  • Analyze PR diff and changed files
  • Review component structure changes
  • Post final review

Clean restoration of the intended architecture — dissolving Common/ and giving each utility component its own focused directory. The changes are surgical and consistent throughout.

Verified:

  • ✅ Zero remaining @/components/Common imports across the entire codebase
  • EmptyButton, CrossButton, GridCenter each in their own directory with a proper index.ts
  • FormRoot, Error, FormElementContainer collocated in FormContainer/ and re-exported from its index.ts
  • BaseButton duplicate in Common/ removed — SplitButton correctly redirected to @/components/Button/BaseButton
  • EllipsisContainer removal is safe — local variants in Collapsible/IconWrapper.tsx and EllipsisContent/EllipsisContent.tsx are independent inline definitions, not consumers of the deleted Common/EllipsisContainer
  • src/index.ts public API is unchanged — no unintended exports added

💡 Suggestions (non-blocking, pre-existing)

  • FormContainer/index.ts:4: export { Error } shadows the JS built-in Error constructor. No public API impact since it's not re-exported from src/index.ts, but it could cause confusion for consumers of the FormContainer module. A rename to FieldError or moving to a FormContainer/internal.ts file would make intent clearer.

  • SplitButton/SplitButton.tsx:4: Imports BaseButton via @/components/Button/BaseButton directly, bypassing Button/index.ts. Consider exporting BaseButton from Button/index.ts to make it part of the module's public contract. (Pre-existing — not introduced here.)


✅ Looks great — clean, minimal, and correct. Nice work! 🙌

@workflow-authentication-public
Copy link
Contributor

📚 Storybook Preview Deployed

✅ Preview URL: https://click-2c6dbn23r-clickhouse.vercel.app

Built from commit: 90cbe0eddb0085038dac6135d3fdb8f8b5f93b58

@punkbit punkbit merged commit d4624f1 into main Mar 6, 2026
8 checks passed
@punkbit punkbit deleted the fix/restore-changes-lost-in-PR-resolution-841-845-merge-conflict-resolution branch March 6, 2026 18:44
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.

3 participants