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: restructure alert system for default integration and Livewire compatibility #78

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

Conversation

ser-tec
Copy link
Contributor

@ser-tec ser-tec commented Dec 6, 2024

Problem

This PR addresses Issue #77, which describes a compatibility problem between the Tablar alert system and Livewire. The session-based flash messages were not displayed during Livewire's render cycle, causing alerts to fail.

Solution

  • Refactored the alert system to include dynamic handling of Livewire events.
  • Implemented a new alerts-container.blade.php for rendering alerts dynamically.
  • Enhanced JavaScript to listen for alert events and manage alert display.
  • Updated tablar.php configuration for additional options like positions, duration, and dismissibility.
  • Ensured backward compatibility with session-based alerts.

Related Issue

Resolves #77


How to Use the Updated Alert System

This update introduces a more flexible alert system compatible with Livewire, while maintaining full backward compatibility. Here’s how it works:

1. Enable Alerts in Configuration

In your config/tablar.php file, ensure that alerts are enabled:

'display_alert' => true,

You can also customize the default alert behavior:

'alerts' => [
    'default_position' => [
        'top' => '10px',
        'right' => '10px',
    ],
    'default_duration' => 5, // Duration in seconds
    'dismissible' => true,   // Alerts can be dismissed by default
],

2. Alert Container

The alert container is already included in page.blade.php:

@if(config('tablar.display_alert'))
    @include('tablar::partials.common.alerts-container')
@endif

If your project uses a custom layout and does not rely on page.blade.php, you must include the container manually in your layout file:

@if(config('tablar.display_alert'))
    @include('tablar::partials.common.alerts-container')
@endif

3. Backward Compatibility with Static Alerts

This system is fully backward compatible with static session-based alerts. You can use them as before:

Session::flash('success', 'Operation completed successfully.');

Static alerts will inherit the default configuration values (e.g., dismissibility and duration). If you want to customize these values, you can pass an array instead of a string:

Session::flash('success', [
    'message' => 'Operation completed successfully.',
    'dismissible' => false, // This alert cannot be dismissed
    'duration' => 10,       // This alert will last for 10 seconds
]);

4. Using Alerts with Livewire

To trigger alerts dynamically from Livewire, dispatch a browser event:

$this->dispatchBrowserEvent('alert', [
    'type' => 'success',          // Alert type (success, error, info, warning)
    'message' => 'Action completed successfully.',
    'dismissible' => true,        // Optional: Default is true
    'duration' => 5,              // Optional: Duration in seconds (default is 5)
]);

5. Error Handling

If there are validation errors, they will automatically be displayed as alerts. Ensure you handle validation errors properly in your controller:

$validated = $request->validate([
    'name' => 'required|string|max:255',
]);
// Validation errors will be displayed as `error` alerts.

Notes on CSS and JavaScript Placement

Currently, the CSS and JavaScript code for the alert system are included directly in resources/views/partials/common/alerts-container.blade.php.

This was done because I don’t know where the maintainers prefer to place these assets. If you can provide specific guidance on where to move the CSS and JavaScript, I will be happy to update the implementation accordingly.

Please let me know where you’d like these assets to be placed, and I’ll make the necessary changes.

…re compatibility

- Added dynamic alert container for handling Livewire events
- Updated configuration for customizable alert positions, duration, and dismissibility
- Improved JavaScript listener for seamless Livewire integration
- Ensured backward compatibility with session-based static alerts
- Removed deprecated `alert.blade.php` view in favor of `alerts-container.blade.php`

This refactor enhances the flexibility and usability of the Tablar alert system while maintaining compatibility with existing functionality.
Copy link
Owner

@takielias takielias left a comment

Choose a reason for hiding this comment

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

Please let me know your update, and I will do further examination. It would be super helpful if you could write some tests for it.

@@ -10,6 +10,9 @@
@section('classes_body', $layoutHelper->makeBodyClasses())

@section('layout')
@if(config('tablar','display_alert'))
Copy link
Owner

Choose a reason for hiding this comment

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

It should have resolved 2 conditions.

  1. when display_alert = true
  2. when livewire = true

It's all about livewire, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @takielias,

Thanks for your feedback! The changes I made are not just about Livewire. I’ve also modified the normal (non-Livewire) usage of alerts to ensure they are always included as an overlay, making them more dynamic and consistent across all use cases.

This means:

When display_alert = true, the alert container will always be rendered, regardless of whether Livewire is being used or not.
For normal (non-Livewire) alerts, they now behave dynamically with improved configuration options (e.g., duration, dismissibility).
For Livewire, the alerts integrate seamlessly using JavaScript events.

If you'd prefer, I can create a separate, more focused PR that resolves the Livewire compatibility issue without modifying the normal alert behavior.

Let me know if this approach works better for you, and I’ll be happy to make the adjustments.

Copy link
Owner

Choose a reason for hiding this comment

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

@ser-tec Thanks for the detailed explanation! For better maintainability and extensibility, would it make sense to keep the concerns separated? I think having a dedicated PR for the Livewire integration would help keep the codebase cleaner. Let me know what you think.

config/tablar.php Show resolved Hide resolved
config/tablar.php Show resolved Hide resolved
@@ -10,6 +10,9 @@
@section('classes_body', $layoutHelper->makeBodyClasses())

@section('layout')
@if(config('tablar','display_alert'))
@include('tablar::partials.common.alerts-container')
Copy link
Owner

Choose a reason for hiding this comment

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

It would be great if you make it common.livewire-alert

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.

[Issue] Problem with Tablar alerts when used with Livewire
2 participants