Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a pinned repositories feature: types and user settings for pinned items, a new usePinnedRepos composable, UI pin/unpin controls in IssueRepoSelect, RepoCard and Sidebar, server search includes repo.fork, i18n keys/schema added, tests for pin behavior, and removal of UToaster from app template. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Repo UI<br/>(IssueRepoSelect / RepoCard / SideBar)
participant Composable as usePinnedRepos<br/>Composable
participant Settings as useUserSettings<br/>API
participant Storage as Persistent<br/>KV Storage
User->>UI: Click pin/unpin
UI->>Composable: toggle(repoFullName, type)
Composable->>Composable: isPinned? / getItem()
alt Not pinned
Composable->>Settings: PUT updated pinnedRepos (add)
Settings->>Storage: persist settings
Storage-->>Settings: ok
else Already pinned
Composable->>Settings: PUT updated pinnedRepos (remove)
Settings->>Storage: persist settings
Storage-->>Settings: ok
end
Settings-->>Composable: return updated settings
Composable-->>UI: computed pinnedRepos updates
UI-->>User: UI reflects new pinned state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/issue/IssueRepoSelect.vue (1)
206-278:⚠️ Potential issue | 🟠 MajorNested
<button>elements (invalid HTML, breaks a11y).Each repo row is a
<button>and now contains aUButton(another button). This is invalid markup and can cause unpredictable click/keyboard behavior. Use a non-button wrapper for the row or render the pin control as a non-button element.✅ Suggested change (apply to both sections)
- <button + <div v-for="repo in filteredOwnRepos" :key="repo.id" class="w-full text-left px-3 py-2.5 hover:bg-elevated transition-colors flex items-start gap-3 cursor-pointer" :class="{ 'bg-elevated/50': repo.fullName === issueStore.selectedRepo }" `@click`="select(repo.fullName)" + role="button" + tabindex="0" + `@keydown.enter.prevent`="select(repo.fullName)" + `@keydown.space.prevent`="select(repo.fullName)" > ... - </button> + </div>Also applies to: 287-348
🤖 Fix all issues with AI agents
In `@app/components/ui/SideBar.vue`:
- Around line 170-172: The pinned section is fully removed by
v-if="pinnedRepos.length && !collapsed", which hides pinned repos when the
sidebar is collapsed; change the template to render the pinned section whenever
pinnedRepos.length is truthy and use the collapsed flag to toggle between full
list and an icon-only/tooltip compact view instead of removing it entirely
(e.g., keep the container with :aria-label="$t('pinnedRepos.pinned')" and
class="mt-2 border-t border-default pt-2 px-1", render the detailed list when
!collapsed and render an icon-only/tooltip button when collapsed so pinnedRepos
remains reachable).
- Around line 40-44: The click handler selectPinnedRepo currently calls
navigateTo('/issues') after updating state, causing duplicate navigation when
the element is already a NuxtLink; remove the navigateTo() call and let NuxtLink
handle navigation (keep issueStore.selectRepo and updateSettings({ selectedRepo
}) in selectPinnedRepo), or alternatively convert the link to a
programmatic-only flow by preventing the NuxtLink behavior and calling
navigateTo() exclusively—apply the same change to the other handler referenced
around selectPinnedRepo (the one at the later block that also calls
navigateTo()).
- Around line 192-193: Guard against malformed repo strings by replacing direct
use of item.repo.split('/')[1] with a safe extractor: add a small computed prop
or helper (e.g., repoLabel or getRepoName) that checks item.repo is a string and
contains '/' and returns the segment after the slash, otherwise returns
item.repo (or an empty string/fallback). Update the template where
item.repo.split('/')[1] is used to render this safe repoLabel so blank labels
are avoided.
| v-if="pinnedRepos.length && !collapsed" | ||
| :aria-label="$t('pinnedRepos.pinned')" | ||
| class="mt-2 border-t border-default pt-2 px-1" |
There was a problem hiding this comment.
Collapsed sidebar hides pinned section entirely (spec mismatch).
Issue #63 calls for an icon-only pinned section when collapsed. The current v-if="pinnedRepos.length && !collapsed" removes it completely, so pinned repos are unreachable while collapsed. Consider rendering a collapsed icon/tooltip state instead.
🤖 Prompt for AI Agents
In `@app/components/ui/SideBar.vue` around lines 170 - 172, The pinned section is
fully removed by v-if="pinnedRepos.length && !collapsed", which hides pinned
repos when the sidebar is collapsed; change the template to render the pinned
section whenever pinnedRepos.length is truthy and use the collapsed flag to
toggle between full list and an icon-only/tooltip compact view instead of
removing it entirely (e.g., keep the container with
:aria-label="$t('pinnedRepos.pinned')" and class="mt-2 border-t border-default
pt-2 px-1", render the detailed list when !collapsed and render an
icon-only/tooltip button when collapsed so pinnedRepos remains reachable).
There was a problem hiding this comment.
please create a new issue and assign it to me. this is a nice idea (I doesn't forgot it, but I though this is a separate feature)
There was a problem hiding this comment.
🧩 Analysis chain
@Flo0806, I'll create that issue for you right away!
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the collapsed sidebar pinned repos feature
gh issue create \
--title "Pinned repos: show icon-only view when sidebar is collapsed" \
--body "## Description
When the sidebar is collapsed, the pinned repos section is currently hidden entirely. According to the original specification in `#63`, the pinned section should show an icon-only view when collapsed, allowing users to still access their pinned repositories.
## Current behavior
- The pinned repos section uses \`v-if=\"pinnedRepos.length && !collapsed\"\` which completely removes it when collapsed
- Pinned repos become unreachable while the sidebar is collapsed
## Expected behavior
- Render the pinned section whenever there are pinned repos
- Show full list when expanded
- Show icon-only/tooltip compact view when collapsed
- Pinned repos remain accessible in both states
## Related
- Original feature request: `#63`
- Implemented in: `#68`
- Discussion: https://github.com/flumen-dev/flumen.dev/pull/68#discussion_r2808070109
- Requested by: `@Flo0806`" \
--assignee "Flo0806" \
--label "enhancement"
Length of output: 50
@Flo0806, done! I've created issue #69 and assigned it to you: #69
The issue captures the idea of showing an icon-only view for pinned repos when the sidebar is collapsed, so they remain accessible in both states. 👍
| <span class="truncate">{{ item.repo.split('/')[1] }}</span> | ||
| <UBadge |
There was a problem hiding this comment.
Guard against unexpected repo string formats.
item.repo.split('/')[1] renders blank if the value lacks a slash. Safer extraction avoids empty labels.
✅ Suggested change
- <span class="truncate">{{ item.repo.split('/')[1] }}</span>
+ <span class="truncate">{{ item.repo.split('/').slice(-1)[0] || item.repo }}</span>🤖 Prompt for AI Agents
In `@app/components/ui/SideBar.vue` around lines 192 - 193, Guard against
malformed repo strings by replacing direct use of item.repo.split('/')[1] with a
safe extractor: add a small computed prop or helper (e.g., repoLabel or
getRepoName) that checks item.repo is a string and contains '/' and returns the
segment after the slash, otherwise returns item.repo (or an empty
string/fallback). Update the template where item.repo.split('/')[1] is used to
render this safe repoLabel so blank labels are avoided.
There was a problem hiding this comment.
Good catch! But repo comes always from our pin function - Only way to malform it is when manually edit in storage file (and then our server would be hacked for :-) ) We let this for now
There was a problem hiding this comment.
@Flo0806, understood! That makes sense—if the repo names are always controlled by your pin function, the risk is indeed minimal. Thanks for the context! 🐰
1 participant
Summary
Related issue(s)
Closes #63
Type of change
Checklist
Summary by CodeRabbit
New Features
Refactor
Tests