Quote #7
Quote #7
Conversation
| @@ -130,18 +130,43 @@ public function last(Store $settings, $slug = '', $id = 0) | |||
| } | |||
| } | |||
|
|
|||
| public function reply($slug = '', $id = 0) | |||
| public function reply($slug = '', $id = 0, $postId = 0) | |||
There was a problem hiding this comment.
How come we've got default values here? Shouldn't the default value for $postId be null?
There was a problem hiding this comment.
$slug and $id are required anyways (Laravel should throw an error already as the route requires two parameters). But whether 0 or null doesn't make any difference as long as the if below is correct. Which should check $postId > 0 instead of only $postId
There was a problem hiding this comment.
Ok, if $slug and $id are required then they shouldn't have default values. I think null is more correct for the default value of $postId and we should keep the if ($postId) check as it is. 0 could conceivably be a valid ID whereas null is a type that is reserved specifically to denote nothingness.
- add Quote Renderer - use PostPresenter - use object instead of array
|
TODO: |
|
I'm not sure if this is relevant to implementing this PR, but my intent in the prototypes is for the quote button to act like multi-quote in 1.x -- quoted posts get added to the quick reply box. This implementation is fine for non-JS, but multi-quote should be added on top of the quote button (not as a separate button). |
|
can you merge it? |
|
Are you asking me about the button or are you asking about this PR? |
|
about this PR! :D about the button: I think we should add a click event with |
|
Click event with return false is what I was suggesting. :-) |
|
Quote Render should have an interface which is injected to the controller and bind in the service provider as markdown has another syntax for quotes |
|
Something like that? |
|
Yep. Atm we can bind the MyCode presenter directly in the service provider as we haven't included any options for markdown so far but as it's planned we should keep that already in mind. |
|
@ATofighi can you add the interface? |
|
@JN-Jones Yes, I'll add it soon... |
|
Is this now complete? |
|
yes |
|
Consider it done 😄 |
No description provided.