Skip to content

Commit

Permalink
Feature: Clickable h4 elements (#4507)
Browse files Browse the repository at this point in the history
<!-- Thank you for taking the time to contribute to The Odin Project. In
order to get this pull request (PR) merged in a reasonable amount of
time, you must complete this entire template. -->

## Because
<!-- Summarize the purpose or reasons for this PR, e.g. what problem it
solves or what benefit it provides. -->
`h4` headings were generating IDs but were not clickable like with `h3`
headings.


## This PR
<!-- A bullet point list of one or more items describing the specific
changes. -->
- Makes level 4 headings render `h4 > a` like with level 3 headings
- Moves the generated IDs to the heading elements themselves, instead of
the parent `section`
- Replaces `section` `id` attribute with `data-title`
- Amends CSS and Tailwind accordingly
- Amends toc controller accordingly
- Amends rspec tests to account for new HTML structure
- Allows `data-title` to render in preview component
- Reduces duplication in a method


## Issue
<!--
If this PR closes an open issue in this repo, replace the XXXXX below
with the issue number, e.g. Closes #2013.

If this PR closes an open issue in another TOP repo, replace the #XXXXX
with the URL of the issue, e.g. Closes
https://github.com/TheOdinProject/curriculum/issues/XXXXX

If this PR does not close, but is related to another issue or PR, you
can link it as above without the 'Closes' keyword, e.g. 'Related to
#2013'.
-->
Closes #4503

## Additional Information
<!-- Any other information about this PR, such as a link to a Discord
discussion. -->


## Pull Request Requirements
<!-- Replace the whitespace between the square brackets with an 'x',
e.g. [x]. After you create the PR, they will become checkboxes that you
can click on. -->
- [x] I have thoroughly read and understand [The Odin Project
Contributing
Guide](https://github.com/TheOdinProject/theodinproject/blob/main/CONTRIBUTING.md)
- [x] The title of this PR follows the `keyword: brief description of
change` format, using one of the following keywords:
    - `Feature` - adds new or amends existing user-facing behavior
- `Chore` - changes that have no user-facing value, refactors,
dependency bumps, etc
    - `Fix` - bug fixes
-   [x] The `Because` section summarizes the reason for this PR
- [x] The `This PR` section has a bullet point list describing the
changes in this PR
- [x] I have verified all tests and linters pass after making these
changes.
- [x] If this PR addresses an open issue, it is linked in the `Issue`
section
-   [x] If applicable, this PR includes new or updated automated tests
  • Loading branch information
MaoShizhong authored Apr 20, 2024
1 parent 8f7507e commit b79afd5
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 35 deletions.
4 changes: 2 additions & 2 deletions app/assets/stylesheets/stylesheets/lesson-content.css
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@
@apply hover:no-underline;
}

.lesson-content h3 > a {
.lesson-content :is(h3, h4) > a {
color:inherit !important;
}

.lesson-content section[id] {
.lesson-content :is(h3, h4)[id] {
scroll-margin-top: 40px;
}

Expand Down
2 changes: 1 addition & 1 deletion app/components/markdown/preview_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def allowed_tags
end

def allowed_attributes
Rails::HTML5::SafeListSanitizer::DEFAULT_ALLOWED_ATTRIBUTES + %w[id]
Rails::HTML5::SafeListSanitizer::DEFAULT_ALLOWED_ATTRIBUTES + %w[id data-title]
end

private
Expand Down
6 changes: 3 additions & 3 deletions app/javascript/controllers/lesson_toc_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default class LessonTocController extends Controller {
{ rootMargin: '-15% 0px -80% 0px' },
);

this.lessonContentTarget.querySelectorAll('section[id]').forEach((section) => {
this.lessonContentTarget.querySelectorAll('section[data-title]').forEach((section) => {
this.observer.observe(section);
});
}
Expand All @@ -27,8 +27,8 @@ export default class LessonTocController extends Controller {

activeSection(entries) {
entries.forEach((entry) => {
const id = entry.target.getAttribute('id');
const tocItem = this.tocTarget.querySelector(`li a[href="#${id}"]`)?.parentElement;
const { title } = entry.target.dataset;
const tocItem = this.tocTarget.querySelector(`li a[href="#${title}"]`)?.parentElement;
if (!tocItem) return;

if (entry.intersectionRatio > 0) {
Expand Down
2 changes: 1 addition & 1 deletion lib/kramdown/content_section.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def initialize(title:, header_location: nil, content: nil)
end

def content
"<section id='#{title.parameterize}' markdown='1'>#{formatted_content}</section>"
"<section data-title='#{title.parameterize}' markdown='1'>#{formatted_content}</section>"
end

def start
Expand Down
10 changes: 5 additions & 5 deletions lib/kramdown/converter/odin_html.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
module Kramdown
module Converter
module OdinHtml
LEVEL_THREE_HEADER = 3
HEADER_LEVELS_TO_CONVERT = [3, 4].freeze
EXTERNAL_LINK_ATTRIBUTES = { target: '_blank', rel: 'noopener noreferrer' }.freeze

def convert_img(element, _indent)
Expand All @@ -24,11 +24,11 @@ def convert_a(element, indent)
end

def convert_header(element, indent)
if element.options[:level] == LEVEL_THREE_HEADER
section_anchor = "##{generate_id(element.options[:raw_text]).parameterize}"
body = "<a#{html_attributes({ href: section_anchor, class: 'anchor-link' })}>#{inner(element, indent)}</a>"
if HEADER_LEVELS_TO_CONVERT.include?(element.options[:level])
heading_id = generate_id(element.options[:raw_text]).parameterize
body = "<a#{html_attributes({ href: "##{heading_id}", class: 'anchor-link' })}>#{inner(element, indent)}</a>"

format_as_block_html('h3', element.attr, body, indent)
format_as_block_html("h#{element.options[:level]}", { id: heading_id }, body, indent)
else
super
end
Expand Down
12 changes: 5 additions & 7 deletions lib/kramdown/document_sections.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,11 @@ def sectionable_content
sections.map.with_index do |section, index|
next_section = sections[index + 1]

if next_section.present?
Kramdown::ContentSection.new(
title: section.title, content: content_between(section.start, next_section.end_of_previous_section)
)
else
Kramdown::ContentSection.new(title: section.title, content: content_between(section.start, END_OF_CONTENT))
end
section_end = next_section&.end_of_previous_section || END_OF_CONTENT

Kramdown::ContentSection.new(
title: section.title, content: content_between(section.start, section_end)
)
end
end

Expand Down
4 changes: 2 additions & 2 deletions spec/lib/kramdown/content_section_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
section = described_class.new(title: 'section title', content: "some markdown\n")

expect(section.content).to eq(
"<section id='section-title' markdown='1'>some markdown\n</section>"
"<section data-title='section-title' markdown='1'>some markdown\n</section>"
)
end

Expand All @@ -17,7 +17,7 @@
section = described_class.new(title: 'section title', content: 'some markdown')

expect(section.content).to eq(
"<section id='section-title' markdown='1'>some markdown\n</section>"
"<section data-title='section-title' markdown='1'>some markdown\n</section>"
)
end
end
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/kramdown/document_sections_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@

it 'creates sections that contain the source content' do
result = <<~SECTIONS.strip
<section id='first-header' markdown='1'>### First header
<section data-title='first-header' markdown='1'>### First header
Test first header
</section><section id='second-header' markdown='1'>### Second header
</section><section data-title='second-header' markdown='1'>### Second header
Test second header
### nested header
</section><section id='third-header' markdown='1'>### Third header
</section><section data-title='third-header' markdown='1'>### Third header
Test third header
</section>
SECTIONS
Expand Down
22 changes: 11 additions & 11 deletions spec/services/markdown_converter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@
MARKDOWN

result = <<~HTML
<section id="first-section-header">
<h3><a href="#first-section-header" class="anchor-link">First section header</a></h3>
<section data-title="first-section-header">
<h3 id="first-section-header"><a href="#first-section-header" class="anchor-link">First section header</a></h3>
<p>some content</p>
</section>
<section id="second-section-header">
<h3><a href="#second-section-header" class="anchor-link">Second section header</a></h3>
<section data-title="second-section-header">
<h3 id="second-section-header"><a href="#second-section-header" class="anchor-link">Second section header</a></h3>
<p>some content</p>
</section>
<section id="third-section-header">
<h3><a href="#third-section-header" class="anchor-link">Third section header</a></h3>
<section data-title="third-section-header">
<h3 id="third-section-header"><a href="#third-section-header" class="anchor-link">Third section header</a></h3>
<p>some content</p>
</section>
HTML
Expand All @@ -46,13 +46,13 @@
MARKDOWN

html_result = <<~HTML
<section id="content">
<section data-title="content">
<h1 id="unsectionable-header">Unsectionable Header</h1>
<p>some content</p>
</section>
<section id="sectionable-header">
<h3><a href="#sectionable-header" class="anchor-link">Sectionable Header</a></h3>
<section data-title="sectionable-header">
<h3 id="sectionable-header"><a href="#sectionable-header" class="anchor-link">Sectionable Header</a></h3>
<p>some content</p>
</section>
HTML
Expand Down Expand Up @@ -129,8 +129,8 @@
MARKDOWN

html_result = <<~HTML
<section id="its-a-header">
<h3><a href="#its-a-header" class="anchor-link">It’s a header</a></h3>
<section data-title="its-a-header">
<h3 id="its-a-header"><a href="#its-a-header" class="anchor-link">It’s a header</a></h3>
<p>content</p>
</section>
HTML
Expand Down
5 changes: 5 additions & 0 deletions tailwind.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ module.exports = {
}
},
},
h4: {
a: {
'text-decoration': 'none',
},
},
details: {
summary: {
'font-size': '1.25rem',
Expand Down

0 comments on commit b79afd5

Please sign in to comment.