Conversation
Mitcheljager
left a comment
Mitcheljager
left a comment
There was a problem hiding this comment.
Good effort, but unfortunately the AI made a real mess of this. When using AI to write your code, you should be careful to only push code that is actually useful and that you understand. As is this introduces an awful lot of ugliness that will need to be cleaned up before this can be merged.
| @posts = @user.posts.select_overview_columns.public?.order("#{ allowed_sort_params.include?(params[:sort_posts]) ? params[:sort_posts] : "created_at" } DESC").page(params[:page]) | ||
| @blocks = Block.where(user_id: @user.id, content_type: :profile).order(position: :asc, created_at: :asc) | ||
| @collections = @user.collections.includes(:posts).where("posts_count > ?", 0) | ||
| @updates = Revision.where(post_id: @user.posts.pluck(:id)).order(created_at: :desc).limit(50) |
There was a problem hiding this comment.
This has a few issues:
- This will include posts that are private or in draft
This will load posts with n+1 because each revision will need to query it's post. It's not an n+1 query, but it is still quite inefficient- A limit of 50 is really quite high, that's a lot of revisions to load at once. Better might be to limit it to like 20 at a time with pagination
| @@ -0,0 +1,88 @@ | |||
| <%# app/views/blocks/profile/_update_log.html.erb %> | |||
There was a problem hiding this comment.
We know what file we're in, we really don't need that at the top
| @@ -0,0 +1,88 @@ | |||
| <%# app/views/blocks/profile/_update_log.html.erb %> | |||
| <div class="mt-1/1"> | |||
There was a problem hiding this comment.
Other profile tabs have a title at the top, it would be nice if this had that too
| @@ -0,0 +1,88 @@ | |||
| <%# app/views/blocks/profile/_update_log.html.erb %> | |||
| <div class="mt-1/1"> | |||
| <% if @updates&.any? %> | |||
There was a problem hiding this comment.
@updates can't be nil since .where will fall back to an empty collection if nothing is found, so the optional chaining here is irrelevant
| <%# app/views/blocks/profile/_update_log.html.erb %> | ||
| <div class="mt-1/1"> | ||
| <% if @updates&.any? %> | ||
| <%# Removed the 'standout' and 'p-0' classes to remove the box/border %> |
There was a problem hiding this comment.
This comment is odd, why do you want me to remove these classes?
| <% end %> | ||
| </div> | ||
|
|
||
| <%# Keep your Styles and Scripts at the bottom as before %> |
There was a problem hiding this comment.
No, don't keep it here. Javascript and css go into their own respective locations and files.
| <%= render "blocks/profile/user_collections" %> | ||
| </div> | ||
| </div> | ||
| </div> No newline at end of file |
There was a problem hiding this comment.
This has a trailing space but no newline at the end of the file
There was a problem hiding this comment.
Please don't include changes in package-lock, these seem to be accidental
| <%# Use 'feed__item' to ensure the site's global CSS handles the horizontal layout %> | ||
| <div class="feed__item <%= "border-top" unless index == 0 %> py-1/2"> | ||
|
|
||
| <%# Left Column: Thumbnail and Code %> | ||
| <div class="feed__info"> | ||
| <%= render "posts/thumbnail", post: revision.post %> | ||
| <%= render "posts/code", post: revision.post, item_class: "code feed__code" %> | ||
|
|
||
| <p class="feed__author"> | ||
| by <%= link_to revision.post.user.clean_username, user_profile_path(revision.post.user) %> | ||
| </p> | ||
|
|
||
| <p class="feed__date"> | ||
| <%= time_ago_in_words(revision.created_at) %> ago | ||
| </p> | ||
| </div> | ||
|
|
||
| <%# Right Column: Text Content (Left-Aligned by default in 'feed__item') %> | ||
| <div class="feed__content"> | ||
| <h3 class="feed__title"> | ||
| <%= link_to revision.post.title, post_path(revision.post.code) %> | ||
| </h3> | ||
|
|
||
| <% if revision.version.present? %> | ||
| <p class="feed__version">v<%= revision.version %></p> | ||
| <% end %> | ||
|
|
||
| <% if revision.description.blank? %> | ||
| <p class="mb-0 text-muted italic"> | ||
| No update notes provided. | ||
| </p> | ||
| <% else %> | ||
| <h5 class="feed__update-notes mt-1/2 mb-1/8">Update notes</h5> | ||
| <div class="revision-description" data-role="expandable-content"> | ||
| <%# 'item__description' ensures the markdown text aligns left %> | ||
| <div class="item__description mt-1/8 line-clamp-3" data-target="expandable-text"> | ||
| <%= sanitized_markdown(revision.description) %> | ||
| </div> | ||
|
|
||
| <% if revision.description.length > 200 %> | ||
| <button class="text-primary text-xs font-bold mt-1/4 p-0 bg-transparent border-0 cursor-pointer hover-underline" | ||
| data-action="toggle-expand"> | ||
| Read more | ||
| </button> | ||
| <% end %> | ||
| </div> | ||
| <% end %> | ||
| </div> | ||
| </div> | ||
| <% end %> | ||
| </div> |
There was a problem hiding this comment.
This is largely a copy of _feed_item.html.erb. Rather than copy pasting the same code, you can reuse that partial with <%= render "feed/feed_item", revision: revision %>. That way you prevent having to make changes in multiple places. Everything re-using the same file.
|
|
||
| <script> | ||
| document.querySelectorAll('[data-action="toggle-expand"]').forEach(button => { | ||
| button.addEventListener('click', (e) => { |
There was a problem hiding this comment.
This event won't get cleaned up when navigating which is problematic with Turbolinks. If you navigate away from this page the event listeners will stay in memory. If you navigate back to this page these event listeners will be attached again over top the other ones. Go back and forth a few times and each button will fire many times over, making it impossible to toggle the content here.
| @posts = @user.posts.select_overview_columns.public?.order("#{ allowed_sort_params.include?(params[:sort_posts]) ? params[:sort_posts] : "created_at" } DESC").page(params[:page]) | ||
| @blocks = Block.where(user_id: @user.id, content_type: :profile).order(position: :asc, created_at: :asc) | ||
| @collections = @user.collections.includes(:posts).where("posts_count > ?", 0) | ||
| @updates = Revision.where(post_id: @user.posts.pluck(:id)).order(created_at: :desc).limit(50) |
There was a problem hiding this comment.
One thing I forgot to mention earlier, this will slow down all profiles even if the feed is never shown. This would be better to load async via a partial just like how tabs are loaded on post pages.
2 participants
added a update log to the profile page of the user.