-
Notifications
You must be signed in to change notification settings - Fork 0
feat(shell): version switcher, archived banner, noindex — #61 phase 4 #69
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -10,9 +10,6 @@ module DocsUI | |||||
| # Every kwarg defaults, so even a naive `entry.view_class.new` (a custom | ||||||
| # registry predating #renderable) renders an empty page rather than raising. | ||||||
| # | ||||||
| # NOTE (issue #61 phase 4): the "you are viewing the 1.0 docs" banner with a | ||||||
| # link to the current equivalent lands with the version switcher, not here. | ||||||
| # | ||||||
| # Deliberately does NOT include Phlex::Rails::Helpers::Routes/Request — their | ||||||
| # bodies run Rails.* at class load, which would make this class (and | ||||||
| # everything referencing it, like Snapshot::Entry#view_class) unloadable in a | ||||||
|
|
@@ -28,16 +25,53 @@ def view_template | |||||
| render DocsUI::Shell.new(title: @entry&.title) { body } | ||||||
| end | ||||||
|
|
||||||
| # The masthead + Markdown body — separated from the Shell wrapper so it can | ||||||
| # render (and be specced) without a Rails view context, the same seam as | ||||||
| # Shell's own topbar/theme-script specs. | ||||||
| # The banner + masthead + Markdown body — separated from the Shell wrapper | ||||||
| # so it can render (and be specced) without a Rails view context, the same | ||||||
| # seam as Shell's own topbar/theme-script specs. | ||||||
| def body | ||||||
| banner | ||||||
| render DocsUI::Header.new(@entry.title) if @entry&.title | ||||||
| render DocsUI::Markdown.new(markdown_source) unless markdown_source.empty? | ||||||
| end | ||||||
|
|
||||||
| private | ||||||
|
|
||||||
| # The "you are viewing archived docs" banner, linking the same slug in the | ||||||
| # current version (falling back to the docs home when the page no longer | ||||||
| # exists there). data-md-skip drops it from the Markdown twin — it's chrome, | ||||||
| # not page content. Absent for a current-version entry (a snapshot of the | ||||||
| # current release rendered directly) and for entries carrying no version. | ||||||
| def banner | ||||||
| version = entry_version | ||||||
| return unless version&.archived? | ||||||
|
|
||||||
| current = DocsKit.configuration.current_version | ||||||
| div(data: { md_skip: true }) do | ||||||
| render DocsUI::Callout.new(:warning) do | ||||||
| plain "You are viewing the #{version.label} docs." | ||||||
| if current | ||||||
| plain " The current version is #{current.label} — " | ||||||
| a(href: current_equivalent_href, class: "link") { "read it there" } | ||||||
| plain "." | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| def entry_version | ||||||
| @entry.version if @entry.respond_to?(:version) | ||||||
| end | ||||||
|
|
||||||
| # The current-version page with this entry's slug, or the docs home when | ||||||
| # the slug has no current equivalent (a page removed since this version). | ||||||
| def current_equivalent_href | ||||||
| config = DocsKit.configuration | ||||||
| slug = @entry.respond_to?(:slug) ? @entry.slug : nil | ||||||
| live = slug && DocsKit::LlmsText.pages(config, version: config.current_version) | ||||||
| .find { |page| page.slug.to_s == slug.to_s } | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Archived pages 500 when the current registry entry lacks Prompt for AI agents
Suggested change
|
||||||
| live&.href || config.brand_href | ||||||
| end | ||||||
|
|
||||||
| def markdown_source | ||||||
| @markdown_source ||= @entry ? @entry.markdown.to_s : "" | ||||||
| end | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -161,6 +161,9 @@ def topbar | |
| end | ||
| render DocsUI::SearchBox.new if config.search_enabled? | ||
| div(class: "flex-none items-center") do | ||
| # The docs-version switcher (config.versions) renders first; nothing | ||
| # unless versioning is enabled, so an unversioned topbar is unchanged. | ||
| render DocsUI::VersionSwitcher.new | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The added VersionSwitcher render now executes before DocsUI::TopbarLinks, so the sibling comment "render as icon-only ghost buttons BEFORE the switcher" is no longer accurate — the topbar links actually render after the switcher. Update that comment (e.g. to "after the switcher") to avoid misleading future readers about render order. Prompt for AI agents |
||
| # Config-driven repo/social links (config.topbar_links) render as | ||
| # icon-only ghost buttons BEFORE the switcher; nothing when unset. | ||
| render DocsUI::TopbarLinks.new | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module DocsUI | ||
| # The topbar documentation-version switcher — the DocsUI::ThemeSwitcher | ||
| # dropdown pattern (tabindex/role=button + dropdown-content): daisyUI's | ||
| # dropdown opens on CSS :focus-within, so it works with JavaScript off, and | ||
| # every entry is a plain <a> — no Stimulus controller (the ONE-controller | ||
| # rule). | ||
| # | ||
| # Renders NOTHING unless config.versioning_enabled? (two or more configured | ||
| # versions), so an unversioned site's topbar is byte-identical to before. | ||
| # | ||
| # Each link targets the SAME slug in the target version when that page exists | ||
| # there, falling back to the target version's first page — a slug missing | ||
| # from an older snapshot must never link a 404. | ||
| class VersionSwitcher < Phlex::HTML | ||
| include Phlex::Rails::Helpers::Request | ||
|
|
||
| def view_template | ||
| return unless config.versioning_enabled? | ||
|
|
||
| div(class: "dropdown dropdown-end", data: { testid: "version-switcher" }) do | ||
| div(tabindex: "0", role: "button", class: "btn btn-sm btn-ghost gap-1") do | ||
| render DocsUI::Icon.new("layers", class: "size-4") | ||
| plain scope_version.label | ||
| end | ||
| ul(tabindex: "0", | ||
| class: "dropdown-content bg-base-300 rounded-box z-10 w-44 p-2 shadow-2xl") do | ||
| config.versions.each { |version| version_option(version) } | ||
| end | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def config = DocsKit.configuration | ||
|
|
||
| # The version this render serves: the request scope, else the current | ||
| # version (versioning_enabled? guarantees one exists — with none marked | ||
| # current, the first configured entry is it). | ||
| def scope_version | ||
| DocsKit::Scope.version || config.current_version | ||
| end | ||
|
Comment on lines
+38
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Normalize the fallback current version.
Either normalize the fallback version to
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
| def version_option(version) | ||
| in_scope = version.id == scope_version&.id | ||
| li do | ||
| a( | ||
| href: target_href(version), | ||
| class: "btn btn-sm btn-block btn-ghost justify-start", | ||
| aria_current: (in_scope ? "true" : nil) | ||
| ) { version.label } | ||
| end | ||
| end | ||
|
|
||
| # The same slug in the target version when it exists there; else the | ||
| # target's first page (guaranteed routable — never a 404); else the | ||
| # target-prefixed docs root (an empty snapshot is already a degraded state). | ||
| def target_href(version) | ||
| pages = DocsKit::LlmsText.pages(config, version: version) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Every topbar render scans the full page list for each configured version just to decide whether a candidate slug exists there (then typically keeps only Prompt for AI agents |
||
| candidate = candidate_href(version) | ||
| return candidate if candidate && pages.any? { |page| page.href == candidate } | ||
|
|
||
| pages.first&.href || "#{version.path_prefix}/docs" | ||
| end | ||
|
|
||
| # The current request path re-prefixed for the target version: strip the | ||
| # in-scope version's prefix, add the target's. nil without a request. | ||
| def candidate_href(version) | ||
| path = current_path | ||
| return unless path | ||
|
|
||
| "#{version.path_prefix}#{path.delete_prefix(DocsKit::Scope.path_prefix)}" | ||
| end | ||
|
|
||
| # The request path, nil when rendered without a live request (an isolated | ||
| # render, a static build) — the DocsUI::Sidebar#current_path guard. | ||
| def current_path | ||
| request&.path | ||
| rescue StandardError | ||
| nil | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,6 +174,34 @@ def image_url(path) = "https://d.example.com/assets/#{path.sub('.png', '-abc123. | |
| expect(render_tags).to include('<meta name="robots" content="noindex, nofollow">') | ||
| end | ||
|
|
||
| describe "robots under a version scope" do | ||
| it "emits noindex, follow for an archived version (canonical untouched)" do | ||
| archived = DocsKit::DocVersion.new(id: "1.0") | ||
|
|
||
| html = DocsKit::Scope.with(version: archived) { render_tags } | ||
|
|
||
| expect(html).to include('<meta name="robots" content="noindex, follow">') | ||
| expect(html).not_to include('rel="canonical"') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: This assertion doesn't actually verify the behavior the test/comment claims ('canonical untouched'). In the isolated render there is no Prompt for AI agents |
||
| end | ||
|
|
||
| it "keeps today's behavior for the current version in scope (regression pin)" do | ||
| current = DocsKit::DocVersion.new(id: "1.1", current: true) | ||
|
|
||
| html = DocsKit::Scope.with(version: current) { render_tags } | ||
|
|
||
| expect(html).not_to include('name="robots"') | ||
| end | ||
|
|
||
| it "restores seo.robots for a version with noindex: false" do | ||
| DocsKit.configure { |c| c.seo.robots = "index, follow" } | ||
| opted_out = DocsKit::DocVersion.new(id: "1.0", noindex: false) | ||
|
|
||
| html = DocsKit::Scope.with(version: opted_out) { render_tags } | ||
|
|
||
| expect(html).to include('<meta name="robots" content="index, follow">') | ||
| end | ||
| end | ||
|
|
||
|
Comment on lines
+177
to
+204
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 8 'render_tags|site_url|original_url|canonical' spec/docs_ui/meta_tags_spec.rbRepository: mhenrixon/docs-kit Length of output: 11750 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
fd -t f -i 'meta_tags|scope|doc_version' .
printf '%s\n' '--- relevant definitions and call sites ---'
rg -n -C 12 'def canonical_url|canonical_url|class MetaTags|module MetaTags|def render_tags|site_url|original_url' .Repository: mhenrixon/docs-kit Length of output: 29225 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
spec = Path("spec/docs_ui/meta_tags_spec.rb").read_text()
impl = Path("app/components/docs_ui/meta_tags.rb").read_text()
helper = re.search(
r'def render_tags\(.*?\n end',
spec,
re.S,
)
assert helper, "render_tags helper not found"
helper_text = helper.group(0)
archived = re.search(
r'it "emits noindex, follow for an archived version.*?\n end',
spec,
re.S,
)
assert archived, "archived example not found"
archived_text = archived.group(0)
canonical = re.search(
r'def canonical_url\n(.*?)\n end',
impl,
re.S,
)
assert canonical, "canonical_url not found"
canonical_body = canonical.group(1)
print("render_tags helper configures site_url:", "site_url" in helper_text)
print("archived example configures site_url:", "site_url" in archived_text)
print("canonical_url returns configured site_url:", "return seo.site_url if seo.site_url" in canonical_body)
print("canonical_url omits URL when request? is false:", "return unless request?" in canonical_body)
assert "site_url" not in helper_text
assert "site_url" not in archived_text
assert "return seo.site_url if seo.site_url" in canonical_body
assert "return unless request?" in canonical_body
PYRepository: mhenrixon/docs-kit Length of output: 347 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
spec = Path("spec/docs_ui/meta_tags_spec.rb").read_text()
impl = Path("app/components/docs_ui/meta_tags.rb").read_text()
helper = re.search(r'def render_tags\(.*?\n end', spec, re.S)
archived = re.search(
r'it "emits noindex, follow for an archived version.*?\n end',
spec,
re.S,
)
canonical = re.search(r'def canonical_url\n(.*?)\n end', impl, re.S)
assert helper and archived and canonical
print("render_tags helper configures site_url:", "site_url" in helper.group(0))
print("archived example configures site_url:", "site_url" in archived.group(0))
print("canonical_url uses site_url:", "return seo.site_url if seo.site_url" in canonical.group(1))
print("canonical_url requires request otherwise:", "return unless request?" in canonical.group(1))
PYRepository: mhenrixon/docs-kit Length of output: 327 Configure a canonical source in the archived-version example.
🤖 Prompt for AI Agents |
||
| it "emits <meta name=\"theme-color\"> only when config.seo.theme_color is set" do | ||
| html = render_tags | ||
| expect(html).not_to include("theme-color") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,34 @@ def view_template = topbar | |
| end | ||
| end | ||
|
|
||
| # The version switcher sits in the topbar right before the repo/social links. | ||
| # It renders NOTHING unless versioning is enabled, so an unversioned site's | ||
| # topbar stays byte-identical. | ||
| describe "the topbar version switcher" do | ||
| let(:topbar_only) do | ||
| Class.new(described_class) do | ||
| def view_template = topbar | ||
| end | ||
| end | ||
|
|
||
| it "renders no switcher on an unversioned site (the byte-identical pin)" do | ||
| html = topbar_only.new.call | ||
|
|
||
| expect(html).not_to include("version-switcher") | ||
| end | ||
|
|
||
| it "renders the switcher when two or more versions are configured" do | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The version-switcher spec covers only the unconfigured (0 versions) and two-version cases, but the behavior's real boundary is a single configured version: Prompt for AI agents |
||
| DocsKit.configure do |c| | ||
| c.versions = [{ id: "1.1", current: true }, { id: "1.0" }] | ||
| end | ||
|
|
||
| html = topbar_only.new.call | ||
|
|
||
| expect(html).to include("1.1") | ||
| expect(html).to include("1.0") | ||
| end | ||
| end | ||
|
|
||
| # The opt-in brand mark (config.brand_logo) — rendered inside the brand anchor | ||
| # in place of the text brand. Absent config → the text brand, byte-identical | ||
| # to before. config.topbar_brand = :mobile_only additionally hides the topbar | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: The effective current version can show an archived banner and link back to itself when no configured version is marked
current: true; compare the entry withDocsKit.configuration.current_versionbefore rendering the banner.Prompt for AI agents