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

disable read all button if it has no unread messages #2534

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

Conversation

swarakaka
Copy link
Contributor

No description provided.

@@ -56,6 +61,9 @@ public function query(Request $request): iterable
->paginate(10);

$this->isNotEmpty = $notifications->isNotEmpty();

$unreadNotifications = $request->user()->unreadNotifications;
$this->hasUnread = boolval(count($unreadNotifications));
Copy link
Member

Choose a reason for hiding this comment

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

I think we can write it down better as:

$request->user()->unreadNotifications()->exists()

But it still doesn't take type into account. Because there can be different types of notification in the application. And also use the automatic filling of public properties.

I think it should look something like this:

/**
 * @var bool
 */
public $isNotEmpty = false;

/**
 * @var bool
 */
public $hasUnread = false;

/**
 * Query data.
 *
 * @return array
 */
public function query(Request $request): iterable
{
    /** @var \Illuminate\Pagination\LengthAwarePaginator $notifications */
    $prepare = $request->user()
        ->notifications()
        ->where('type', DashboardMessage::class);

    $notifications = $prepare->paginate(10);

    return [
        'notifications' => $notifications,
        'isNotEmpty'    => $notifications->total() > 0,
        'hasUnread'     => $prepare->unread()->exists(),
    ];
}

/**
 * Button commands.
 *
 * @return Action[]
 */
public function commandBar(): iterable
{
    return [
        Button::make(__('Remove All'))
            ->icon('trash')
            ->method('removeAll')
            ->confirm(__('After deleting notifications, this action cannot be undone and all associated data will be permanently lost.'))
            ->disabled(!$this->isNotEmpty),

        Button::make(__('Mark All As Read'))
            ->icon('eye')
            ->method('markAllAsRead')
            ->disabled(!$this->hasUnread),
    ];
}

What do you think about it?

Copy link
Contributor Author

@swarakaka swarakaka Mar 1, 2023

Choose a reason for hiding this comment

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

However, the values of the $isNotEmpty and $hasUnread variables do not change within the query, that is, they always return the base value and other variables are created. So both buttons will always remain disabled.

solution:

  public function query(Request $request): iterable
    {
        /** @var \Illuminate\Pagination\LengthAwarePaginator $notifications */
        $prepare = $request->user()
            ->notifications()
            ->where('type', DashboardMessage::class);

        $notifications           = $prepare->paginate(10);
        $this->isNotEmpty  = $notifications->total() > 0;
        $this->hasUnread   = $prepare->unread()->exists();

        return [
            'notifications' => $notifications,
        ];
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants