Support flagging comments using Markable#60
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves how comment flagging is handled by implementing custom flag counting and state management to ensure that only one flag per user exists while enhancing the UI responsiveness and form layout. Key changes include:
- Refactored flag components with client-side state management and loading indicators.
- Updated model methods for accurate flag counting and user flag detection.
- Improved styling for buttons and flag forms to address layout and icon display issues.
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/views/livewire/flags/flag-component.blade.php | Refactored the flag form with enhanced identifiers, loading state handling, and event-based UI interactions. |
| resources/views/livewire/comments/partials/toggle-flagging-button.blade.php | Updated the toggle button to include loading state and revised icon usage. |
| resources/views/livewire/comments/comment-show-component.blade.php | Adjusted flag removal logic and corrected markup structure. |
| resources/views/livewire/comments/comment-component.blade.php | Streamlined flag state updates by removing redundant properties and using updated methods for flag counts. |
| resources/sass/modules/_posts.scss | Enhanced button icon display based on loading state. |
| resources/sass/modules/_forms.scss | Revised form styling with flexbox for radio labels and updated textarea resize rules. |
| app/Traits/CommentComponentTrait.php | Added flag loading state resets in flag start/stop methods. |
| app/Models/Post.php | Implemented custom flag counting and user flag detection methods. |
| app/Models/Comment.php | Renamed the flag count method and added a user flag check. |
| app/Livewire/Flags/FlagComponent.php | Refactored flag logic with locked properties, improved state management, and event dispatch enhancements. |
| app/Livewire/Comments/CommentComponent.php | Optimized flag update logic and removed obsolete flag handling methods. |
| app/Enums/LivewireEventEnum.php | Added new events for flag cancellation in both comment and post contexts. |
Comments suppressed due to low confidence (1)
resources/views/livewire/flags/flag-component.blade.php:32
- Consider using a sanitized identifier (such as the loop index or a slugified version) instead of the raw '$reason' value for the 'id' attribute to ensure uniqueness and valid HTML.
<label for="{{ $type }}-{{ $model->id }}-flag-note-{{ $reason }}" class="optional">
| name="selectedReason" | ||
| wire:model="selectedReason" | ||
| wire:change="flagReasonSelected('{{ $reason }}')" | ||
| wire:click="showNoteField = {{ json_encode($this->isNoteVisibleForReason($reason)) }}" |
There was a problem hiding this comment.
Consider using a dedicated method call to toggle the note field visibility instead of an inline assignment to improve readability and testability.
| wire:click="showNoteField = {{ json_encode($this->isNoteVisibleForReason($reason)) }}" | |
| wire:click="toggleNoteField('{{ $reason }}')" |
resources/sass/modules/_forms.scss
Outdated
| } | ||
|
|
||
| .flag-form textarea.flag-note { | ||
| resize: block; |
There was a problem hiding this comment.
The CSS property value 'resize: block;' is invalid; consider using a valid value like 'vertical' or 'none' based on the intended behavior.
| resize: block; | |
| resize: vertical; |
There was a problem hiding this comment.
I'm not convinced this was wrong, but MDN was calling out support for block separately to the other resize options, so I suppose vertical might be more widely supported. I don't see any notes on this on caniuse.com either, so 🤷
wrose504
force-pushed
the
comment-flagging
branch
4 times, most recently
from
1fafed8 to
12ab4fb
Compare
| public function flagCount(): int | ||
| { | ||
| return Flag::count($this); | ||
| return Flag::where([ | ||
| 'markable_id' => $this->getKey(), | ||
| 'markable_type' => $this->getMorphClass(), | ||
| ])->count(); | ||
| } | ||
|
|
||
| public function userFlagged(): bool | ||
| { | ||
| return Flag::where([ | ||
| 'user_id' => auth()->id(), | ||
| 'markable_id' => $this->getKey(), | ||
| 'markable_type' => $this->getMorphClass(), | ||
| ])->exists(); | ||
| } |
There was a problem hiding this comment.
We could try to move these methods more generically into a trait as countAll and userMarked or something (or a dynamic name). I think the downside is we wouldn't be able to use the Bookmark and Favorite marks directly from laravel-markable, we'd probably have to create a subclass.
wrose504
force-pushed
the
comment-flagging
branch
3 times, most recently
from
6a0c6b0 to
1ca3676
Compare
wrose504
force-pushed
the
comment-flagging
branch
from
1ca3676 to
777ba62
Compare
wrose504
force-pushed
the
comment-flagging
branch
from
777ba62 to
09d1d9e
Compare
Comment flagging is not working properly at present for a few reasons:
laravel-markabletrait to count all flags irrespective of reason (aka value), so we have to implement this ourselves.This PR tries to fix several of these issues:
I mostly tested this on a branch with #51, #52 and #59 all merged, so it would be better to merge those first then resolve the conflicts with this branch.
Before
After