Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧹 Journal: Move Entry rendering to EntryComponent #1650

Merged
merged 1 commit into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/furniture/journal/entries/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
<%- breadcrumb :journal_entry, entry %>
<%= render entry %>
<%= render Journal::EntryComponent.new(entry: entry) %>
16 changes: 0 additions & 16 deletions app/furniture/journal/entry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ class Entry < ApplicationRecord
location(parent: :journal)

self.table_name = "journal_entries"
include RendersMarkdown
extend StripsNamespaceFromModelName

scope :recent, -> { order("published_at DESC NULLS FIRST") }
Expand Down Expand Up @@ -38,21 +37,6 @@ def published?
published_at.present?
end

def to_html
render_markdown(body)
end

def self.renderer
@_renderer ||= Redcarpet::Markdown.new(
Renderer.new(filter_html: true, with_toc_data: true),
autolink: true, strikethrough: true,
no_intra_emphasis: true,
lax_spacing: true,
fenced_code_blocks: true, disable_indented_code_blocks: true,
tables: true, footnotes: true, superscript: true, quote: true
)
end

def extract_keywords
self.keywords = journal.keywords.extract_and_create_from!(body).pluck(:canonical_keyword)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
<header class="border-b border-primary-500">
<h2 class=" text-primary-900"><%= entry.headline %></h2>
<%- if entry.published?%>
<em>Published on <%= link_to entry.published_at.to_fs(:long_ordinal), entry.location %></em>
<em>Published on <%= link_to published_at, entry.location %></em>
<%- else %>
<em>Unpublished.</em>
<%- end %>
</header>

<div class="prose">
<%= entry.to_html.html_safe %>
<%== body_html %>
</div>

<footer class="flex justify-between my-2">
Expand Down
30 changes: 30 additions & 0 deletions app/furniture/journal/entry_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
class Journal
class EntryComponent < ApplicationComponent
include RendersMarkdown
attr_accessor :entry

def initialize(*args, entry:, **kwargs)
self.entry = entry
super(*args, **kwargs)
end

def body_html
Copy link
Contributor

Choose a reason for hiding this comment

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

What about content_html not to confuse with the<body> tag. This might be more consistent with other components that define header/content/footer slots?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I like that! I will do a follow up PR to rename Entry#body to Entry#content

render_markdown(entry.body)
end

def published_at
entry.published_at.to_fs(:long_ordinal)
end

def self.renderer
@_renderer ||= Redcarpet::Markdown.new(
Renderer.new(filter_html: true, with_toc_data: true),
autolink: true, strikethrough: true,
no_intra_emphasis: true,
lax_spacing: true,
fenced_code_blocks: true, disable_indented_code_blocks: true,
tables: true, footnotes: true, superscript: true, quote: true
)
end
end
end
4 changes: 2 additions & 2 deletions app/furniture/journal/journals/_journal.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
<%- end %>

<div class="flex flex-wrap">
<%- @pagy, @records = pagy(policy_scope(journal.entries.recent)) %>
<%- @pagy, @entries = pagy(policy_scope(journal.entries.recent)) %>

<%= render @records %>
<%= render Journal::EntryComponent.with_collection(@entries) %>
</div>

<%== pagy_nav(@pagy, nav_extra: 'flex justify-between') %>
Expand Down
10 changes: 10 additions & 0 deletions spec/furniture/journal/entry_component_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
require "rails_helper"

RSpec.describe Journal::EntryComponent, type: :component do
subject(:output) { render_inline(component) }

let(:component) { described_class.new(entry: entry) }
let(:entry) { create(:journal_entry, body: "https://www.google.com @[email protected] #GoodTimes") }

it { is_expected.to have_link("https://www.google.com", href: "https://www.google.com") }
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason you're just testing the link existence and not other strings?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I could have done some tests to ensure headings have the appropriate ids and such; but this was the thrust of the work I was doing so it's what I focused on.

end
10 changes: 0 additions & 10 deletions spec/furniture/journal/entry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,6 @@
it { is_expected.to have_one(:room).through(:journal) }
it { is_expected.to have_one(:space).through(:journal) }

describe "#to_html" do
subject(:to_html) { entry.to_html }

context "when #body is 'https://www.google.com @[email protected]'" do
let(:entry) { build(:journal_entry, body: "https://www.google.com @[email protected]") }

it { is_expected.to include('<a href="https://www.google.com">https://www.google.com</a>') }
end
end

describe "#save" do
let(:entry) { create(:journal_entry, body: "#GoodTimes") }
let(:journal) { entry.journal }
Expand Down