feat: fold hierarchical tags with '/' in Versions tab#32
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds hierarchical tag/tree rendering for version references: detects tags containing '/', renders grouped collapsible headers and grouped items via a new tree render flow, preserves flat rendering otherwise, and updates selection to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
spoorcc
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dfetch_hub/site/index.html (1)
891-899:⚠️ Potential issue | 🟠 MajorSelected version can stay hidden after URL/popstate selection in tree mode.
selectVersion(i)updates highlight only; it doesn’t re-open the group containing that row. If selection is restored programmatically, the selected tag can remain collapsed/invisible.Suggested fix
function selectVersion(i){ const r=allRefsCache[i];if(!r)return; curVersion={name:r.name,kind:r.kind}; _replaceURL({pkg:curPkg,v:r.name,vk:r.kind}); - document.querySelectorAll('#vtbody tr[data-vidx]').forEach(tr=>tr.classList.toggle('ver-selected',+tr.dataset.vidx===i)); + const hasHierTags=allRefsCache.some(ref=>ref.kind==='tag'&&ref.name.includes('/')); + if(hasHierTags){ + document.getElementById('vtbody').innerHTML=_renderVerTree(allRefsCache); + }else{ + document.querySelectorAll('#vtbody tr[data-vidx]') + .forEach(tr=>tr.classList.toggle('ver-selected',+tr.dataset.vidx===i)); + } const c=curPkg?RAW[curPkg]:null; if(c)document.getElementById('dSnip').innerHTML=buildSnipHtml(c,curVersion); updSelVerInfo(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dfetch_hub/site/index.html` around lines 891 - 899, selectVersion(i) only toggles the selected row class and doesn't ensure the tree/group containing that row is expanded, so programmatic restores can leave the selected row hidden; inside selectVersion (which reads allRefsCache and sets curVersion), find the corresponding row element (the tr with data-vidx === i) and expand/un-collapse its parent group(s) before toggling 'ver-selected' — e.g., traverse up from that tr to any group container elements or collapse toggles and set them to expanded (remove collapsed class or set aria-expanded/open as used by the tree implementation) so the selected row becomes visible (optionally call scrollIntoView if desired). Ensure this logic references selectVersion, allRefsCache, and the row selector '#vtbody tr[data-vidx]'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dfetch_hub/site/index.html`:
- Line 920: The row-level toggle (vtree-group-hdr) currently only uses an
onclick on the <tr> so it isn’t keyboard accessible; update the markup and
handlers around toggleTreeGroup to make the control focusable and operable via
keyboard: replace or augment the clickable <tr> content with a focusable element
(e.g., a <button> or a span with role="button" and tabindex="0") containing the
chevron and prefix, set an appropriate aria-expanded attribute reflecting open,
and wire the same toggleTreeGroup logic to that element’s click and keydown
(handle Enter/Space) events; apply the same change for the other group header
occurrences handled by toggleTreeGroup (the similar vtree-group-hdr blocks).
---
Outside diff comments:
In `@dfetch_hub/site/index.html`:
- Around line 891-899: selectVersion(i) only toggles the selected row class and
doesn't ensure the tree/group containing that row is expanded, so programmatic
restores can leave the selected row hidden; inside selectVersion (which reads
allRefsCache and sets curVersion), find the corresponding row element (the tr
with data-vidx === i) and expand/un-collapse its parent group(s) before toggling
'ver-selected' — e.g., traverse up from that tr to any group container elements
or collapse toggles and set them to expanded (remove collapsed class or set
aria-expanded/open as used by the tree implementation) so the selected row
becomes visible (optionally call scrollIntoView if desired). Ensure this logic
references selectVersion, allRefsCache, and the row selector '#vtbody
tr[data-vidx]'.
| // Render grouped tags — collapsed by default, open if selected version is inside | ||
| groups.forEach((idxs,pfx)=>{ | ||
| const open=idxs.some(i=>curVersion&&refs[i].name===curVersion.name&&refs[i].kind===curVersion.kind); | ||
| html+=`<tr class="vtree-group-hdr" onclick="toggleTreeGroup(this)"><td colspan="4"><span class="vtree-chevron">${open?'▾':'▸'}</span><span class="vtree-prefix">${esc(pfx)}/</span><span class="vtree-count">(${idxs.length})</span></td></tr>`; |
Contributor
There was a problem hiding this comment.
Group expand/collapse control is not keyboard accessible.
The toggle is attached to a clickable <tr> only, so keyboard users can’t focus/activate it.
Suggested fix
-.vtree-group-hdr td{background:var(--bg)!important;cursor:pointer;font-size:.8rem;font-weight:700;padding:.45rem .85rem;color:var(--muted);user-select:none;border-bottom:1px solid var(--border)}
+.vtree-group-hdr td{background:var(--bg)!important;font-size:.8rem;font-weight:700;padding:.45rem .85rem;color:var(--muted);user-select:none;border-bottom:1px solid var(--border)}
+.vtree-group-btn{display:flex;align-items:center;gap:0;width:100%;border:none;background:none;padding:0;text-align:left;cursor:pointer;color:inherit;font:inherit}
+.vtree-group-btn:focus-visible{outline:2px solid var(--accent);outline-offset:2px}- html+=`<tr class="vtree-group-hdr" onclick="toggleTreeGroup(this)"><td colspan="4"><span class="vtree-chevron">${open?'▾':'▸'}</span><span class="vtree-prefix">${esc(pfx)}/</span><span class="vtree-count">(${idxs.length})</span></td></tr>`;
+ html+=`<tr class="vtree-group-hdr"><td colspan="4"><button type="button" class="vtree-group-btn" onclick="toggleTreeGroup(this.closest('tr'))"><span class="vtree-chevron">${open?'▾':'▸'}</span><span class="vtree-prefix">${esc(pfx)}/</span><span class="vtree-count">(${idxs.length})</span></button></td></tr>`;Also applies to: 936-942
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dfetch_hub/site/index.html` at line 920, The row-level toggle
(vtree-group-hdr) currently only uses an onclick on the <tr> so it isn’t
keyboard accessible; update the markup and handlers around toggleTreeGroup to
make the control focusable and operable via keyboard: replace or augment the
clickable <tr> content with a focusable element (e.g., a <button> or a span with
role="button" and tabindex="0") containing the chevron and prefix, set an
appropriate aria-expanded attribute reflecting open, and wire the same
toggleTreeGroup logic to that element’s click and keydown (handle Enter/Space)
events; apply the same change for the other group header occurrences handled by
toggleTreeGroup (the similar vtree-group-hdr blocks).
7e4bd84 to
21849ce
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dfetch_hub/site/index.html`:
- Around line 293-299: The CSS rule for .vtree-group-hdr td is too long; break
the single-line declaration into multiple lines so each property is on its own
line (for example, place background, cursor, font-size, font-weight, padding,
color, user-select, border-bottom each on separate lines) while keeping the
selector .vtree-group-hdr td and the !important and values intact; ensure the
rewritten block stays within 120 characters per line and preserves the existing
specificity and declarations used elsewhere (see .vtree-group-hdr td and related
.vtree-group-hdr:hover td for consistent formatting).
- Around line 907-935: The _renderVerTree function has multiple template-literal
lines that exceed the 120-char limit (notably the per-group header, grouped-item
row, and ungrouped-item row); refactor by extracting small helper functions such
as buildGroupHeader(prefix, count, open), buildGroupItem(r, i, sel, isLatest,
pfx), and buildItemRow(r, i, sel, isLatest) (or
buildBadge(kind)/buildDate(date)) and compose the HTML from those shorter
template pieces/variables, or split attributes and content into multiple
concatenated strings so no single line exceeds 120 chars; update calls in
_renderVerTree (where groups.forEach and refs.forEach produce html) to use these
helpers and preserve behavior for toggleTreeGroup/selectVersion and the
data-vidx, vtag-latest, vbadge, vtree-chevron, and vtree-prefix pieces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 657fbae8-3ccb-4f84-9c72-710da5eb3d79
📒 Files selected for processing (1)
dfetch_hub/site/index.html
Comment on lines
+907
to
+935
| function _renderVerTree(refs){ | ||
| // Group tags whose names contain '/' by their prefix (part before first /) | ||
| const groups=new Map(); | ||
| refs.forEach((r,i)=>{ | ||
| if(r.kind!=='tag'||!r.name.includes('/'))return; | ||
| const pfx=r.name.slice(0,r.name.indexOf('/')); | ||
| if(!groups.has(pfx))groups.set(pfx,[]); | ||
| groups.get(pfx).push(i); | ||
| }); | ||
| let html=''; | ||
| // Render grouped tags — collapsed by default, open if selected version is inside | ||
| groups.forEach((idxs,pfx)=>{ | ||
| const open=idxs.some(i=>curVersion&&refs[i].name===curVersion.name&&refs[i].kind===curVersion.kind); | ||
| html+=`<tr class="vtree-group-hdr" onclick="toggleTreeGroup(this)"><td colspan="4"><span class="vtree-chevron">${open?'▾':'▸'}</span><span class="vtree-prefix">${esc(pfx)}/</span><span class="vtree-count">(${idxs.length})</span></td></tr>`; | ||
| idxs.forEach(i=>{ | ||
| const r=refs[i];const sel=curVersion&&r.name===curVersion.name&&r.kind===curVersion.kind; | ||
| const isLatest=i===0; | ||
| html+=`<tr class="vtree-item${sel?' ver-selected':''}" data-vidx="${i}" onclick="selectVersion(${i})"${open?'':' style="display:none"'}><td class="vtag-name">${esc(r.name.slice(pfx.length+1))}${isLatest?'<span class="vtag-latest">latest</span>':''}</td><td><span class="vbadge vbadge-tag">tag</span></td><td class="vsha">${r.commit_sha?r.commit_sha.slice(0,10):'—'}</td><td style="font-size:.79rem;color:var(--subtle)">${r.date?r.date.slice(0,10):'—'}</td></tr>`; | ||
| }); | ||
| }); | ||
| // Render ungrouped tags and branches | ||
| refs.forEach((r,i)=>{ | ||
| if(r.kind==='tag'&&r.name.includes('/'))return; | ||
| const sel=curVersion&&r.name===curVersion.name&&r.kind===curVersion.kind; | ||
| const isLatest=i===0&&r.kind==='tag'; | ||
| html+=`<tr class="${sel?'ver-selected':''}" data-vidx="${i}" onclick="selectVersion(${i})"><td class="vtag-name">${esc(r.name)}${isLatest?'<span class="vtag-latest">latest</span>':''}</td><td><span class="vbadge vbadge-${r.kind}">${r.kind}</span></td><td class="vsha">${r.commit_sha?r.commit_sha.slice(0,10):'—'}</td><td style="font-size:.79rem;color:var(--subtle)">${r.date?r.date.slice(0,10):'—'}</td></tr>`; | ||
| }); | ||
| return html||'<tr><td colspan="4" style="text-align:center;color:var(--subtle)">No version info</td></tr>'; | ||
| } |
Contributor
There was a problem hiding this comment.
Multiple lines exceed 120-character limit.
Lines 920, 924, and 932 contain very long template literals (200-300+ characters). Consider breaking these into multiple statements or helper functions for readability.
As per coding guidelines: "Enforce maximum line length of 120 characters in HTML/CSS/JS".
Example refactor for line 924
- html+=`<tr class="vtree-item${sel?' ver-selected':''}" data-vidx="${i}" onclick="selectVersion(${i})"${open?'':' style="display:none"'}><td class="vtag-name">${esc(r.name.slice(pfx.length+1))}${isLatest?'<span class="vtag-latest">latest</span>':''}</td><td><span class="vbadge vbadge-tag">tag</span></td><td class="vsha">${r.commit_sha?r.commit_sha.slice(0,10):'—'}</td><td style="font-size:.79rem;color:var(--subtle)">${r.date?r.date.slice(0,10):'—'}</td></tr>`;
+ const dispStyle = open ? '' : ' style="display:none"';
+ const latestBadge = isLatest ? '<span class="vtag-latest">latest</span>' : '';
+ const sha = r.commit_sha ? r.commit_sha.slice(0,10) : '—';
+ const date = r.date ? r.date.slice(0,10) : '—';
+ html += `<tr class="vtree-item${sel?' ver-selected':''}" data-vidx="${i}" ` +
+ `onclick="selectVersion(${i})"${dispStyle}>` +
+ `<td class="vtag-name">${esc(r.name.slice(pfx.length+1))}${latestBadge}</td>` +
+ `<td><span class="vbadge vbadge-tag">tag</span></td>` +
+ `<td class="vsha">${sha}</td>` +
+ `<td style="font-size:.79rem;color:var(--subtle)">${date}</td></tr>`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dfetch_hub/site/index.html` around lines 907 - 935, The _renderVerTree
function has multiple template-literal lines that exceed the 120-char limit
(notably the per-group header, grouped-item row, and ungrouped-item row);
refactor by extracting small helper functions such as buildGroupHeader(prefix,
count, open), buildGroupItem(r, i, sel, isLatest, pfx), and buildItemRow(r, i,
sel, isLatest) (or buildBadge(kind)/buildDate(date)) and compose the HTML from
those shorter template pieces/variables, or split attributes and content into
multiple concatenated strings so no single line exceeds 120 chars; update calls
in _renderVerTree (where groups.forEach and refs.forEach produce html) to use
these helpers and preserve behavior for toggleTreeGroup/selectVersion and the
data-vidx, vtag-latest, vbadge, vtree-chevron, and vtree-prefix pieces.
When any tag name contains a forward slash (as commonly seen in monorepo setups, e.g. component/v1.0.0), the Versions tab now renders a collapsible tree instead of a flat list. Tags are grouped by the prefix before the first '/', and each group starts collapsed. Groups auto-expand when the currently selected version lives inside them. - Added CSS for .vtree-group-hdr, .vtree-item, .vtree-chevron, etc. - Added _renderVerTree() to build the grouped HTML - Added toggleTreeGroup() to expand/collapse a group - Updated selectVersion() to use data-vidx attribute for highlight (works in both flat and tree modes) Closes #31
spoorcc
force-pushed
the
claude/solve-issue-31-OpT31
branch
from
21849ce to
8569a59
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dfetch_hub/site/index.html (1)
900-905:⚠️ Potential issue | 🟠 MajorExpand the parent group when restoring a selected hierarchical tag.
Line 904 only updates the selected class. On initial load and
popstate,renderDetail()builds the tree using the default version first, thenselectVersion(i)applies the URL-selected version afterward. If that version is inside a collapsed group, the URL/snippet update but the selected row stays hidden.Suggested fix
function selectVersion(i){ const r=allRefsCache[i];if(!r)return; curVersion={name:r.name,kind:r.kind}; _replaceURL({pkg:curPkg,v:r.name,vk:r.kind}); - document.querySelectorAll('#vtbody tr[data-vidx]').forEach(tr=>tr.classList.toggle('ver-selected',+tr.dataset.vidx===i)); + document.querySelectorAll('#vtbody tr[data-vidx]').forEach(tr=>{ + tr.classList.toggle('ver-selected', +tr.dataset.vidx === i); + }); + const row=document.querySelector(`#vtbody tr[data-vidx="${i}"]`); + if(row&&row.style.display==='none'){ + let hdr=row.previousElementSibling; + while(hdr&&!hdr.classList.contains('vtree-group-hdr'))hdr=hdr.previousElementSibling; + if(hdr){ + const chevron=hdr.querySelector('.vtree-chevron'); + if(chevron)chevron.textContent='▾'; + for(let sib=hdr.nextElementSibling;sib&&sib.classList.contains('vtree-item');sib=sib.nextElementSibling){ + sib.style.display=''; + } + } + } const c=curPkg?RAW[curPkg]:null; if(c)document.getElementById('dSnip').innerHTML=buildSnipHtml(c,curVersion); updSelVerInfo(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dfetch_hub/site/index.html` around lines 900 - 905, selectVersion currently only toggles the 'ver-selected' class but doesn't expand any collapsed parent groups, so a URL-selected hierarchical tag can remain hidden; update selectVersion to, after setting curVersion and calling _replaceURL, locate the selected row (using the same selector used to apply 'ver-selected' — '#vtbody tr[data-vidx]' or the row whose dataset.vidx === i) and walk up to its group/container elements (e.g., ancestor rows or elements with the group identifier/class used in the tree renderer), and programmatically expand them (either by removing the collapsed class/attribute or by calling the existing expand function if one exists) before applying 'ver-selected' so the selected row becomes visible; ensure this runs in selectVersion alongside the existing _replaceURL and curVersion updates and uses the same DOM identifiers to find the row.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@dfetch_hub/site/index.html`:
- Around line 900-905: selectVersion currently only toggles the 'ver-selected'
class but doesn't expand any collapsed parent groups, so a URL-selected
hierarchical tag can remain hidden; update selectVersion to, after setting
curVersion and calling _replaceURL, locate the selected row (using the same
selector used to apply 'ver-selected' — '#vtbody tr[data-vidx]' or the row whose
dataset.vidx === i) and walk up to its group/container elements (e.g., ancestor
rows or elements with the group identifier/class used in the tree renderer), and
programmatically expand them (either by removing the collapsed class/attribute
or by calling the existing expand function if one exists) before applying
'ver-selected' so the selected row becomes visible; ensure this runs in
selectVersion alongside the existing _replaceURL and curVersion updates and uses
the same DOM identifiers to find the row.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf72da6c-2e23-48a9-9095-055c2c6448f1
📒 Files selected for processing (1)
dfetch_hub/site/index.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: fold hierarchical tags with '/' in Versions tab
When any tag name contains a forward slash (as commonly seen in monorepo
setups, e.g. component/v1.0.0), the Versions tab now renders a collapsible
tree instead of a flat list. Tags are grouped by the prefix before the
first '/', and each group starts collapsed. Groups auto-expand when the
currently selected version lives inside them.
(works in both flat and tree modes)
Closes #31
Summary by CodeRabbit