-
Couldn't load subscription status.
- Fork 3
feat: Add DetailHeader #24
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 a new DetailHeader component for the UCLA library's Vue component library, along with supporting utilities and types. The component provides navigation controls for detail pages including previous/next links, results count, tag information, and an optional back button.
Key changes:
- Created DetailHeader component with navigation, results display, and tag information features
- Added utility functions for number formatting and pluralization
- Established TypeScript types for button components
- Updated existing SectionPagination component with formatting improvements
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| DetailHeader.vue | Main component implementing detail page header with navigation and metadata display |
| DetailHeader.stories.js | Storybook stories demonstrating various DetailHeader configurations |
| Button.vue | Reusable button component with multiple variants and states |
| Button.stories.js | Storybook stories for button component variations |
| pluralize.js | Utility functions for proper word pluralization |
| formatNumber.js | Utility for formatting numbers with locale-appropriate separators |
| button.types.ts | TypeScript definitions for button component props and enums |
| _detail-header.scss | Styling for the DetailHeader component |
| _button.scss | Styling for the Button component |
| SectionPagination.vue | Code formatting improvements (quotes and indentation) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| <SvgIconArrowRight class="next-svg" /> | ||
| </SmartLink> | ||
|
|
||
| <div v-if="totalResults" class="results-number"> |
Copilot
AI
Aug 13, 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 condition v-if="totalResults" will hide the results display when totalResults is 0, but 0 is a valid result count that should be displayed. Consider changing to v-if="totalResults !== undefined && totalResults !== null" or v-if="typeof totalResults === 'number'" to show "0 results" when appropriate.
| <div v-if="totalResults" class="results-number"> | |
| <div v-if="typeof totalResults === 'number'" class="results-number"> |
| * formatNumber(1234.56) // "1,234.56" | ||
| */ | ||
| export function formatNumber(num) { | ||
| if (num === null || num === undefined || isNaN(num)) { |
Copilot
AI
Aug 13, 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 isNaN(num) check will return true for non-numeric types like strings, which may not be the intended behavior. Consider using Number.isNaN(num) instead, or add a type check like typeof num !== 'number' to be more explicit about the validation.
| if (num === null || num === undefined || isNaN(num)) { | |
| if (typeof num !== 'number' || Number.isNaN(num)) { |
| </div> | ||
| </div> | ||
|
|
||
| <div class="tag" v-if="tag.name"> |
Copilot
AI
Aug 13, 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 condition v-if="tag.name" could cause a runtime error if tag is undefined or null. Consider adding a null check: v-if="tag?.name" or v-if="tag && tag.name" to prevent accessing properties on undefined objects.
| <div class="tag" v-if="tag.name"> | |
| <div class="tag" v-if="tag && tag.name"> |
|
|
||
| // Types | ||
| interface DetailHeaderProps { | ||
| totalResults?: number |
Copilot
AI
Aug 13, 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 totalResults prop allows negative numbers, which doesn't make sense for a results count. Consider adding validation or using a more restrictive type that ensures non-negative values.
| totalResults?: number |
❌ Deploy Preview for ucla-components-storybook failed. Why did it fail? →
|
Component Created: DetailHeader.vue
Stories: ~/stories/DetailHeader.stories.js
Notes:
This PR uses a button implemented in this PR. So it might be a better idea to review the Button PR first.
Checklist:
library-website-nuxt dev server