- 
                Notifications
    
You must be signed in to change notification settings  - Fork 3
 
feat: Add BlockAnchorNav & PanelAnchorNav #29
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?
Conversation
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.
Pull Request Overview
This PR introduces two new navigation components - BlockAnchorNav and PanelAnchorNav - along with a reusable Button component. The changes include proper TypeScript types, comprehensive Storybook stories with edge case testing, and updates to design tokens for DLC (Digital Library Collections) support.
Key changes:
- Created BlockAnchorNav component for inline anchor navigation with sanitization
 - Created PanelAnchorNav component for slide-out panel navigation with scroll detection
 - Added Button component with primary/secondary variants and multiple color options
 
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description | 
|---|---|
| packages/vue-component-library/src/lib-components/BlockAnchorNav.vue | Main anchor navigation component with input sanitization and validation | 
| packages/vue-component-library/src/lib-components/PanelAnchorNav.vue | Slide-out panel wrapper for BlockAnchorNav with scroll behavior | 
| packages/vue-component-library/src/lib-components/Button.vue | Reusable button component with TypeScript types and variant support | 
| packages/vue-component-library/src/types/components/blockAnchorNav.types.ts | TypeScript interfaces for BlockAnchorNav props | 
| packages/vue-component-library/src/types/components/button.types.ts | TypeScript enums and interfaces for Button component | 
| packages/vue-component-library/src/styles/default/_*.scss | SCSS styling for all three components | 
| packages/vue-component-library/src/stories/*.stories.js | Comprehensive Storybook stories including edge cases | 
| packages/vue-component-library/vite.config.ts | Added DLC design token imports | 
| packages/vue-component-library/package.json | Updated design tokens version | 
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Allow only safe protocols | ||
| const SAFE_PROTOCOLS = ['http:', 'https:', 'mailto:', 'tel:'] | ||
| try { | ||
| const url = new URL(to, 'http://dummy.base') // base needed for relative URLs | 
    
      
    
      Copilot
AI
    
    
    
      Aug 21, 2025 
    
  
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.
Using a dummy base URL could potentially mask malicious URLs. Consider using window.location.origin or a more explicit validation approach instead of a dummy base.
| const url = new URL(to, 'http://dummy.base') // base needed for relative URLs | |
| const url = new URL(to, window.location.origin) // base needed for relative URLs | 
| // Scroll detection | ||
| function handleScroll() { | ||
| if (!props.showOpenIconAlways) { | ||
| // Show open icon after scrolling 50% of the viewport height | 
    
      
    
      Copilot
AI
    
    
    
      Aug 21, 2025 
    
  
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.
The comment mentions 'after scrolling 50% of the viewport height' but the code calculates 50% (0.5), which contradicts the earlier comment about '100vh'. Consider updating the comment to be consistent.
| // Show open icon after scrolling 50% of the viewport height | |
| // Show open icon after scrolling at least 50% of the viewport height | 
| width: 37.5px; | ||
| height: 37.5px; | ||
| 
               | 
          ||
| transform: translateX(100vw); | 
    
      
    
      Copilot
AI
    
    
    
      Aug 21, 2025 
    
  
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.
[nitpick] The transform value translateX(100vw) is inconsistent with other transform values in the file that use percentages. Consider using translateX(100%) for consistency.
| transform: translateX(100vw); | |
| transform: translateX(100%); | 
| 
               | 
          ||
| output.push({ label, to }) | ||
| } | ||
| return output | 
    
      
    
      Copilot
AI
    
    
    
      Aug 21, 2025 
    
  
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.
[nitpick] Consider using Array.filter() and Array.map() instead of a manual for loop for better functional programming style and readability.
| return output | |
| return props.items | |
| .filter((raw: any) => { | |
| const label = typeof raw?.label === 'string' ? raw.label.trim() : ''; | |
| const to = typeof raw?.to === 'string' ? raw.to.trim() : ''; | |
| return label && to && isRouteLike(to); | |
| }) | |
| .map((raw: any) => ({ | |
| label: typeof raw?.label === 'string' ? raw.label.trim() : '', | |
| to: typeof raw?.to === 'string' ? raw.to.trim() : '', | |
| })); | 
| 
               | 
          ||
| // Add CSS for real page scenario | ||
| const style = document.createElement('style') | ||
| style.textContent = ` | 
    
      
    
      Copilot
AI
    
    
    
      Aug 21, 2025 
    
  
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.
Adding styles directly to the document head in a story file could cause side effects between stories. Consider using Storybook's decorator pattern or CSS-in-JS approach instead.
| style.textContent = ` | |
| RealPageWithManySections.decorators = [withRealPageStyle] | |
| // Decorator to inject and clean up styles for the real page scenario | |
| const realPageStyle = ` | 
          ✅ Deploy Preview for ucla-components-storybook ready!
 To edit notification comments on pull requests, go to your Netlify project configuration.  | 
    
Component Created: BlockAnchorNav & PanelAnchorNav.vue
Stories: ~/stories/BlockAnchorNav & PanelAnchorNav.stories.js
Spec: ~/stories/BlockAnchorNav & PanelAnchorNav.spec.js
Notes:


Checklist:
library-website-nuxt dev server