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

Refactor Confirmation Dialogs and Event Handling in Dashboard #730

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tofoli
Copy link

@tofoli tofoli commented Feb 20, 2025

Summary

This pull request refactors the confirmation dialog handling in the Split dashboard. The changes improve the maintainability of the code by removing unused functions and replacing inline event handlers with class-based event listeners.

Changes Made

  • Removed the confirmStep function as it was not utilized.
  • Introduced a centralized event handler setup that listens for click events on elements with specific class names related to various actions (e.g., reopen, enable cohorting, disable cohorting, reset, delete, and confirm winner).
  • Updated the form elements in the _controls.erb and _experiment.erb views to use class names instead of inline onclick attributes for triggering confirmation dialogs.

Benefits

  • Enhances code readability and maintainability.
  • Reduces the amount of inline JavaScript, promoting a cleaner separation of concerns.
  • Simplifies the addition of new confirmation dialogs in the future.

- Removed the unused confirmStep function.
- Added event listeners for confirmation dialogs using class-based selectors for better maintainability.
- Updated form elements to use class names for triggering confirmation dialogs instead of inline onclick attributes.
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.

1 participant