Skip to content
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

Visual Experiments - Add EditDOMMutations modal #1394

Merged
merged 7 commits into from Jun 28, 2023

Conversation

bttf
Copy link
Collaborator

@bttf bttf commented Jun 23, 2023

This is a simple modal available on the Experiments page that allows a user to delete 1. Global CSS 2. Custom JS or 3. Any single DOM mutation from a variation in their Visual Experiment

Closes #1393

Screenshots

Screenshot 2023-06-23 at 12 54 01 PM Screenshot 2023-06-23 at 12 54 04 PM Screenshot 2023-06-23 at 12 54 10 PM Screenshot 2023-06-23 at 12 54 14 PM Screenshot 2023-06-23 at 12 54 20 PM

@bttf bttf requested a review from a team June 23, 2023 19:55
@github-actions
Copy link

github-actions bot commented Jun 23, 2023

Your preview environment pr-1394-bttf has been deployed.

Preview environment endpoints are available at:

visualChange
);

const deleteCustomJS = useCallback(() => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: useCallback for these seems unnecessary, as it's a very inexpensive operation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's fair. It comes from a force of habit. I don't equate useCallback as a thing to use when something is expensive but rather when 'we don't need to redefine this function per render'

@bttf bttf requested a review from a team June 23, 2023 21:26
@bttf
Copy link
Collaborator Author

bttf commented Jun 28, 2023

Thanks @bryce-fitzsimons Largely agree with your feedback. I've updated the UI as follows, let me know what you think

  1. Pencil icon Screenshot 2023-06-27 at 7 55 08 PM

  2. Better delete links Screenshot 2023-06-27 at 7 55 32 PM

@bttf
Copy link
Collaborator Author

bttf commented Jun 28, 2023

Also added this (bypass link)
image

onClick={() => deleteDOMMutation(i)}
style={{ fontSize: "1.2rem", paddingLeft: "0.3rem" }}
>
×
Copy link
Member

Choose a reason for hiding this comment

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

IMO this one looks out of place compared to the other delete links

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does. What about this?
image

Copy link
Member

Choose a reason for hiding this comment

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

Complicated! But good enough I think.

Copy link
Member

@bryce-fitzsimons bryce-fitzsimons left a comment

Choose a reason for hiding this comment

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

lgtm

@bttf bttf merged commit 5db4cc2 into main Jun 28, 2023
3 checks passed
@bttf bttf deleted the adnan/delete-visual-change-modal branch June 28, 2023 17:48
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.

[Bug] Visual Editor - Custom JS - Redirects cause the visual editor to be inaccessible
2 participants