Add a 'Jump to latest' button in mobile and desktop view and also adj…#39
Conversation
app/views/topics/show.html.slim
Outdated
| .thread-actions data-controller="thread-actions" [email protected] data-thread-actions-read-all-url-value=read_all_topic_path(@topic, format: :json) | ||
| button.button-secondary data-action="click->thread-actions#markAllRead" Mark all messages read | ||
| - last_message = @messages.last | ||
| - if user_signed_in? || last_message.present? |
There was a problem hiding this comment.
I would make this user_signed_in? && last_message.present?
and then the last if, which isn't dependent on the user, only last_message, could be one lever higher.
Overall a lot simpler because we need less nested conditions.
There was a problem hiding this comment.
Can make it easier, but don't we have three conditions?
- both buttons when user_signed_in? && last_message.present?
- only “Mark all messages read” when signed in
- only “Jump to latest” when not signed in
There was a problem hiding this comment.
First button: both conditions
Second button: only the has message condition
And the "thread action attrs" require no additional condition, that was just part of the mark all read button and worked fine previously
There was a problem hiding this comment.
(strictly speaking we never display threads with 0 messages, so we don't really need the "has message" condition, but it's a good safety net)
There was a problem hiding this comment.
Gotcha, adjusted - please check again
…ust the style of merge topic and this new button to match the existing style, so it's clear these are actual buttons Signed-off-by: Kai Wagner <[email protected]>
ImTheKai
force-pushed
the
jump-to-latest
branch
from
66ed6d8 to
502ed1d
Compare
2 participants
…ust the style of merge topic and this new button to match the existing style, so it's clear these are actual buttons