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
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Platform/Http/Screens/NotificationScreen.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ class NotificationScreen extends Screen
*/
private $isNotEmpty = false;

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

/**
* Query data.
*
Expand All @@ -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,
        ];
    }


return [
'notifications' => $notifications,
Expand All @@ -79,6 +87,7 @@ public function commandBar(): iterable
Button::make(__('Mark all as read'))
->icon('eye')
->method('markAllAsRead')
->disabled(!$this->hasUnread)
->canSee($this->isNotEmpty),
];
}
Expand Down