Skip to content

Conversation

@smartlime
Copy link
Owner

  1. Не стал создавать вложенный контроллер треков, но, по идее, было бы чище.
  2. "Матрешка" из фреймов все равно получается, и без нее выйдет только через стрим, как мне кажется.
  3. Не стал сильно красиво делать пагинацию, по идее, не суть.
  4. Пренебрег тем, что после последней страницы будет пустой запрос.

Copy link

@palkan palkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не стал создавать вложенный контроллер треков, но, по идее, было бы чище.

Определённо)

Ну да ладно, по основной задаче всё сделано.

<span class="track--number--text track--playing-indicator"></span>
<% else %>
<%= track_counter + 1 %>
<%= offset + track_counter + 1 %>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,12 @@
<%# locals: (artist:, tracks:, page: params[:page].to_i) -%>
<%= turbo_frame_tag dom_id(artist, :tracks) do %>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кажется, тут не хватает target — что будет с кликом по треку?

<ul class="tracks">
<%= render tracks, enqueueble: true, offset: page * 10 %>
</ul>
<%= turbo_frame_tag dom_id(artist, :tracks), target: "_self",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Интересная идея с target: "_self", но тот же вопрос про клики по трекам остается. Можно, конечно, представить, что мы их уже переделали на стримы.. Но сейчас явно будет глючить.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Дааа. из-за target-ов глючит нещадно! Действительно, клики... Попробую на досуге доработать, кажется, что тут еще надо подумать, как упаковать фреймы.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants