Skip to content

Commit

Permalink
Refactor game-state-view containers organization
Browse files Browse the repository at this point in the history
This is a follow-up on my original restructuring (back in
cf21337) after playing with it for a
bit. I was having pain with using `show` pages as both a template and
a "partial".

e.g. having to do `<%# locals: (view: @view) %>` in the
games/new.html.erb template was hard to reason about. Templates
shouldn't have local variables, and especially not *sometimes*. This
just adds to cognitive load.

So now, we have a convention of using a "container" (partial + view
model) instead.
  • Loading branch information
Paul DobbinSchmaltz committed Nov 10, 2024
1 parent 9015ccc commit 921051d
Show file tree
Hide file tree
Showing 24 changed files with 101 additions and 90 deletions.
2 changes: 1 addition & 1 deletion app/channels/war_room_channel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def log_subscription_event(method_name) # rubocop:disable Metrics/MethodLength
end

def broadcast_subscription_update
Games::Current::Show.broadcast_players_count_update(
Games::Current::Container.broadcast_players_count_update(
stream_name:, count: DutyRoster.count)
end
end
17 changes: 17 additions & 0 deletions app/views/games/current/_container.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<%# locals: (container:) %>
<% content_for(:page_nav) do %>
<%= render("games/current/page_nav", nav: container.nav) %>
<% end %>
<%= turbo_frame_tag(container.turbo_frame_name, class: "space-y-12") do %>
<% container.content.tap do |content| %>
<%= turbo_frame_tag(content.turbo_frame_name) do %>
<%= render("games/current/content", content:) %>
<% end %>
<% end %>

<div class="container mx-auto">
<%= render("games/current/footer", footer: container.footer(self)) %>
</div>
<% end %>
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# frozen_string_literal: true

# Games::Current::Show represents the entire view context surrounding the
# current {Game}.
class Games::Current::Show
# Games::Current::Container represents the entire view context surrounding the
# current {Game}, as a partial for reuse.
class Games::Current::Container
def self.broadcast_players_count_update(stream_name:, count:)
count = [count, 1].max
WarRoomChannel.broadcast_update_to(
Expand All @@ -15,8 +15,8 @@ def initialize(current_game:)
@current_game = current_game
end

def template_path
"games/current/show"
def partial_path
"games/current/container"
end

def turbo_frame_name = self.class.turbo_frame_name
Expand Down
17 changes: 0 additions & 17 deletions app/views/games/current/show.html.erb

This file was deleted.

13 changes: 13 additions & 0 deletions app/views/games/just_ended/_container.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<%# locals: (container:) %>
<%= turbo_frame_tag(container.turbo_frame_name) do %>
<div class="space-y-12">
<%= render("games/just_ended/content", content: container.content) %>

<div class="container mx-auto">
<%= render(
"games/just_ended/footer",
footer: container.footer(user: current_user)) %>
</div>
</div>
<% end %>
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
# frozen_string_literal: true

# Games::JustEnded::Show represents the entire view context surrounding {Game}s
# that have just ended.
class Games::JustEnded::Show
# Games::JustEnded::Container represents the entire view context surrounding
# {Game}s that have just ended, as a partial for reuse.
class Games::JustEnded::Container
def initialize(current_game:)
@current_game = current_game
end

def template_path
"games/just_ended/show"
def partial_path
"games/just_ended/container"
end

def turbo_frame_name = Games::Current::Show.turbo_frame_name
def turbo_frame_name = Games::Current::Container.turbo_frame_name

def content
Games::JustEnded::Content.new(current_game:)
Expand Down
13 changes: 0 additions & 13 deletions app/views/games/just_ended/show.html.erb

This file was deleted.

4 changes: 2 additions & 2 deletions app/views/games/just_ended/update.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ def initialize(current_game:)
@current_game = current_game
end

def show
Games::JustEnded::Show.new(current_game:)
def container
Games::JustEnded::Container.new(current_game:)
end

private
Expand Down
8 changes: 4 additions & 4 deletions app/views/games/just_ended/update.turbo_stream.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<% @view.show.tap do |show| %>
<% @view.container.tap do |container| %>
<%= turbo_stream.update(
show.turbo_frame_name,
template: "games/just_ended/show",
locals: { view: show }) %>
container.turbo_frame_name,
partial: "games/just_ended/container",
locals: { container: }) %>
<% end %>
10 changes: 2 additions & 8 deletions app/views/games/new.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
<%# locals: (view: @view) %>
<% title("New Game") %>
<% title("New Game") unless title %>

<div class="container mx-auto md:mt-24 space-y-16">
<h1 class="text-center"><%= t("game.new_html").sample %></h1>

<%= render("games/new/content", content: view.content) %>
</div>
<%= render("games/new/container", container: @view.container) %>
8 changes: 2 additions & 6 deletions app/views/games/new.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@

# Games::New is represents New {Game}s.
class Games::New
def template_path
"games/new"
end

def content
Games::New::Content.new
def container
Games::New::Container.new
end
end
7 changes: 7 additions & 0 deletions app/views/games/new/_container.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<%# locals: (container:) %>

<div class="container mx-auto md:mt-24 space-y-16">
<h1 class="text-center"><%= t("game.new_html").sample %></h1>

<%= render("games/new/content", content: container.content) %>
</div>
13 changes: 13 additions & 0 deletions app/views/games/new/container.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

# Games::New::Container represents the entire view context surrounding new
# {Game}s, as a partial for reuse.
class Games::New::Container
def partial_path
"games/new/container"
end

def content
Games::New::Content.new
end
end
13 changes: 13 additions & 0 deletions app/views/games/past/_container.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<%# locals: (container:) %>
<%= turbo_frame_tag(container.turbo_frame_name) do %>
<div class="space-y-12">
<% cache_unless(App.debug?, container.cache_key(context: layout)) do %>
<%= render("games/past/content", content: container.content) %>
<% end %>

<div class="container mx-auto">
<%= render("games/past/footer", footer: container.footer) %>
</div>
</div>
<% end %>
2 changes: 1 addition & 1 deletion app/views/games/past/active_link_behaviors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ def link_action
end

def turbo_frame_name
Games::Past::Show.display_case_turbo_frame_name
Games::Past::Container.display_case_turbo_frame_name
end
end
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# frozen_string_literal: true

# Games::Past::Show represents the entire view context surrounding past {Game}s.
class Games::Past::Show
# Games::Past::Container represents the entire view context surrounding past
# {Game}s, as a partial for reuse.
class Games::Past::Container
def self.display_case_turbo_frame_name = :past_game_display_case

def initialize(game:)
Expand Down
2 changes: 1 addition & 1 deletion app/views/games/past/display_case_behaviors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ def container_css
end

def turbo_frame_name
Games::Past::Show.display_case_turbo_frame_name
Games::Past::Container.display_case_turbo_frame_name
end
end
13 changes: 0 additions & 13 deletions app/views/games/past/show.html.erb

This file was deleted.

2 changes: 1 addition & 1 deletion app/views/games/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
<% end %>
<% end %>
<%= render(template: "games/past/show", locals: { view: @view.show }) %>
<%= render("games/past/container", container: @view.container) %>
4 changes: 2 additions & 2 deletions app/views/games/show.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ def nav
Games::Past::Nav.new(game:)
end

def show
Games::Past::Show.new(game:)
def container
Games::Past::Container.new(game:)
end

private
Expand Down
4 changes: 2 additions & 2 deletions app/views/home/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@
<% end %>
<% end %>
<% @view.view.tap do |view| %>
<%= render(template: view.template_path, locals: { view: }) %>
<% @view.container.tap do |container| %>
<%= render(container.partial_path, container:) %>
<% end %>
8 changes: 4 additions & 4 deletions app/views/home/show.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ def welcome_banner(context:)
Home::WelcomeBanner.new(context:)
end

def view
def container
if !current_game
Games::New.new
Games::New::Container.new
elsif current_game.on?
Games::Current::Show.new(current_game:)
Games::Current::Container.new(current_game:)
elsif current_game.just_ended?
Games::JustEnded::Show.new(current_game:)
Games::JustEnded::Container.new(current_game:)
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/metrics/games/show.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def initialize(game:, user:)

def game_number = game.display_id

def turbo_frame_name = Games::Past::Show.display_case_turbo_frame_name
def turbo_frame_name = Games::Past::Container.display_case_turbo_frame_name

def cache_key(context:)
[
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/games/show.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def nav
Users::Games::Nav.new(game:, user:)
end

def turbo_frame_name = Games::Past::Show.display_case_turbo_frame_name
def turbo_frame_name = Games::Past::Container.display_case_turbo_frame_name

def cache_key(context:)
[
Expand Down

0 comments on commit 921051d

Please sign in to comment.