-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(no-code experiments): refactor toolbar (1) #28206
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR implements the first phase of refactoring the no-code experiments toolbar, focusing on improving content editing and state management.
- Enforces mutually exclusive text/HTML editing in
WebExperimentTransformField.tsx
to prevent content override conflicts - Added proper state tracking via
original_html_state
inWebExperimentForm
for undo functionality - Improved state management using Kea reducers instead of direct mutations in
experimentsTabLogic.tsx
- Updated type definitions to make variants required and conditions optional in
types.ts
- Potential issue:
addNewElement
still initializes both text and html properties which contradicts single-mode editing
4 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
frontend/src/toolbar/experiments/WebExperimentTransformField.tsx
Outdated
Show resolved
Hide resolved
Size Change: +271 B (+0.02%) Total Size: 1.17 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
Problem
First pass at refactoring Toolbar for no-code experiments. Here's the full list of things to do which I'll be adding to.
Changes
textContent
orinnerHTML
, but not both, since they can override each other. Currently,innerHTML
updates last in posthog-js, so it "wins."How did you test this code?
Manually.