Commit all pending 4#5
Conversation
This workflow installs Python dependencies, runs linting with flake8, and executes tests with pytest.
Move CI workflow to standard .github/workflows directory
…ith-radix-ui Add Radix Accordion + cn helper and publish portfolio with styling refinements
Summary of ChangesHello @zubinqayam, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant new features by adding a reusable Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
33c4610
into
copilot/add-github-workflows
There was a problem hiding this comment.
Code Review
This pull request adds several new components and a portfolio page. My review focuses on improving code maintainability, user experience, and accessibility. In lib/utils.ts, I've suggested replacing the custom cn function with standard libraries like clsx and tailwind-merge for better performance and maintainability. For the new portfolio.html page, I've identified several areas for improvement: removing redundant JavaScript for hover effects in favor of a pure CSS approach, addressing a non-functional tel: link, improving HTML semantics by replacing a <br> tag with CSS margins, and enhancing accessibility by adding text alternatives for emoji icons and removing a typewriter effect that can be problematic for screen reader users. The new accordion.tsx component looks good and follows best practices.
| export type ClassValue = string | number | null | false | undefined | ClassDictionary | ClassArray; | ||
|
|
||
| interface ClassDictionary { | ||
| [key: string]: any; | ||
| } | ||
|
|
||
| interface ClassArray extends Array<ClassValue> {} | ||
|
|
||
| function toVal(mix: ClassValue): string { | ||
| let str = ''; | ||
| if (typeof mix === 'string' || typeof mix === 'number') { | ||
| str += mix; | ||
| } else if (Array.isArray(mix)) { | ||
| for (const item of mix) { | ||
| const val = toVal(item); | ||
| if (val) { | ||
| if (str) str += ' '; | ||
| str += val; | ||
| } | ||
| } | ||
| } else if (mix && typeof mix === 'object') { | ||
| for (const key in mix as ClassDictionary) { | ||
| if ((mix as ClassDictionary)[key]) { | ||
| if (str) str += ' '; | ||
| str += key; | ||
| } | ||
| } | ||
| } | ||
| return str; | ||
| } | ||
|
|
||
| export function cn(...inputs: ClassValue[]): string { | ||
| let str = ''; | ||
| for (const input of inputs) { | ||
| const val = toVal(input); | ||
| if (val) { | ||
| if (str) str += ' '; | ||
| str += val; | ||
| } | ||
| } | ||
| return str; | ||
| } |
There was a problem hiding this comment.
The custom cn function is a re-implementation of what's provided by popular, battle-tested libraries like clsx and tailwind-merge. To improve maintainability, reduce the bundle size, and leverage highly-optimized code, it is recommended to use these standard libraries. Using tailwind-merge is especially beneficial as it intelligently handles conflicting Tailwind CSS classes.
| export type ClassValue = string | number | null | false | undefined | ClassDictionary | ClassArray; | |
| interface ClassDictionary { | |
| [key: string]: any; | |
| } | |
| interface ClassArray extends Array<ClassValue> {} | |
| function toVal(mix: ClassValue): string { | |
| let str = ''; | |
| if (typeof mix === 'string' || typeof mix === 'number') { | |
| str += mix; | |
| } else if (Array.isArray(mix)) { | |
| for (const item of mix) { | |
| const val = toVal(item); | |
| if (val) { | |
| if (str) str += ' '; | |
| str += val; | |
| } | |
| } | |
| } else if (mix && typeof mix === 'object') { | |
| for (const key in mix as ClassDictionary) { | |
| if ((mix as ClassDictionary)[key]) { | |
| if (str) str += ' '; | |
| str += key; | |
| } | |
| } | |
| } | |
| return str; | |
| } | |
| export function cn(...inputs: ClassValue[]): string { | |
| let str = ''; | |
| for (const input of inputs) { | |
| const val = toVal(input); | |
| if (val) { | |
| if (str) str += ' '; | |
| str += val; | |
| } | |
| } | |
| return str; | |
| } | |
| import { type ClassValue, clsx } from "clsx"; | |
| import { twMerge } from "tailwind-merge"; | |
| export function cn(...inputs: ClassValue[]) { | |
| return twMerge(clsx(inputs)); | |
| } |
| <div class="container"> | ||
| <h2 class="section-title">About</h2> | ||
| <div class="about-content"> | ||
| <div class="about-image"> |
There was a problem hiding this comment.
The emoji 🏥 is used as a visual icon but lacks an accessible text alternative for screen reader users. This can make the content difficult to understand for visually impaired users. To improve accessibility, provide a text alternative using aria-label and assign a role="img". This feedback applies to all other emojis used as icons throughout the page (e.g., 🏭, 🚢, 🤝, 💡).
| <div class="about-image"> | |
| <div class="about-image" role="img" aria-label="Hospital building icon"> |
| </div> | ||
| <div class="about-text"> | ||
| <p>As a strategic healthcare innovation leader, I specialize in facilitating multinational partnerships that drive Oman's Vision 2040 industrial transformation program. My expertise spans healthcare technology integration, port development initiatives, and strategic industrial development across the Sohar Freezone and broader regional markets.</p> | ||
| <br> |
There was a problem hiding this comment.
| <a href="tel:+968-XXXXXXXX" class="contact-item"> | ||
| <span>📱</span> | ||
| <span>Phone</span> | ||
| </a> |
| // Add hover effects for experience cards | ||
| document.querySelectorAll('.experience-card').forEach(card => { | ||
| card.addEventListener('mouseenter', function() { | ||
| this.style.transform = 'translateY(-8px) scale(1.02)'; | ||
| }); | ||
|
|
||
| card.addEventListener('mouseleave', function() { | ||
| this.style.transform = 'translateY(0) scale(1)'; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This JavaScript block for handling hover effects on .experience-card is redundant, as there is already a CSS :hover rule for this selector on lines 249-252. The JavaScript and CSS also define different transformations, leading to inconsistency. It's best practice to handle static hover effects with CSS for better performance and maintainability. Please remove this JavaScript code.
| // Dynamic typing effect for hero title (optional enhancement) | ||
| function typeWriter(element, text, speed = 100) { | ||
| let i = 0; | ||
| element.innerHTML = ''; | ||
| function type() { | ||
| if (i < text.length) { | ||
| element.innerHTML += text.charAt(i); | ||
| i++; | ||
| setTimeout(type, speed); | ||
| } | ||
| } | ||
| type(); | ||
| } | ||
|
|
||
| // Initialize typing effect on page load | ||
| window.addEventListener('load', () => { | ||
| const heroTitle = document.querySelector('.hero-title'); | ||
| const originalText = heroTitle.textContent; | ||
| typeWriter(heroTitle, originalText, 80); | ||
| }); |
There was a problem hiding this comment.
The typewriter effect implemented here can create accessibility issues. For users of assistive technologies like screen readers, dynamically clearing and typing out text in a heading can be disorienting. The existing fade-in animation on the heading is more accessible. Consider removing this script to ensure a better experience for all users and to simplify the code.
There was a problem hiding this comment.
Pull request overview
This PR adds four disconnected components to the repository: a standalone portfolio HTML page for Zubin Qayam, TypeScript utility functions, a React accordion component, and a Python application CI workflow. The changes appear to be a consolidation of multiple unrelated features.
Changes:
- Added a complete portfolio website as a single HTML file with embedded CSS and JavaScript
- Introduced TypeScript utility functions for CSS class name manipulation
- Added a React accordion component using Radix UI primitives
- Created a GitHub Actions workflow for Python testing and linting
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 16 comments.
| File | Description |
|---|---|
| public/portfolio.html | Complete portfolio website with 600+ lines of HTML, CSS, and JavaScript |
| lib/utils.ts | TypeScript utility for combining CSS class names |
| components/ui/accordion.tsx | React accordion component using Radix UI |
| .github/workflows/python-app.yml | CI workflow for Python linting and testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Add hover effects for experience cards | ||
| document.querySelectorAll('.experience-card').forEach(card => { | ||
| card.addEventListener('mouseenter', function() { | ||
| this.style.transform = 'translateY(-8px) scale(1.02)'; | ||
| }); | ||
|
|
||
| card.addEventListener('mouseleave', function() { | ||
| this.style.transform = 'translateY(0) scale(1)'; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The JavaScript adds inline hover effects that override CSS-defined transitions, which is redundant and can cause maintenance issues. The experience-card already has CSS hover transitions defined at lines 249-251. The JavaScript hover handlers (lines 595-601) will override these CSS transitions, creating inconsistent behavior and making the code harder to maintain. Consider removing the JavaScript hover handlers and relying solely on the CSS :hover rules.
| <div class="container"> | ||
| <h2 class="section-title">About</h2> | ||
| <div class="about-content"> | ||
| <div class="about-image"> |
There was a problem hiding this comment.
The about-image div uses emojis as decorative content but doesn't have proper alt text or aria-label attributes. For accessibility, decorative elements should have aria-hidden="true", or if they're meaningful, they should have appropriate ARIA labels to describe their purpose to screen reader users.
| <div class="about-image"> | |
| <div class="about-image" aria-hidden="true"> |
| for (const key in mix as ClassDictionary) { | ||
| if ((mix as ClassDictionary)[key]) { | ||
| if (str) str += ' '; | ||
| str += key; | ||
| } | ||
| } |
There was a problem hiding this comment.
The for...in loop iterates over all enumerable properties including inherited ones from the prototype chain. This could lead to unexpected behavior if the object has inherited properties. Use Object.keys(), Object.entries(), or hasOwnProperty() to ensure only own properties are iterated.
| @@ -0,0 +1,626 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
The PR description is extremely vague and doesn't explain what changes are being made or why. The title "Commit all pending 4" and description mentioning "Commit and merge to to main all 4 PR" (note the typo "to to") don't provide meaningful context about what this PR accomplishes. A clear description should explain: (1) what functionality is being added, (2) why these changes are needed, and (3) how the different components (portfolio HTML, React components, Python workflow) relate to each other.
| <style> | ||
| :root { | ||
| /* Primitive Color Tokens */ | ||
| --color-white: rgba(255, 255, 255, 1); | ||
| --color-black: rgba(0, 0, 0, 1); | ||
| --color-cream-50: rgba(252, 252, 249, 1); | ||
| --color-cream-100: rgba(255, 255, 253, 1); | ||
| --color-gray-200: rgba(245, 245, 245, 1); | ||
| --color-gray-300: rgba(167, 169, 169, 1); | ||
| --color-gray-400: rgba(119, 124, 124, 1); | ||
| --color-slate-50: rgba(248, 250, 252, 1); | ||
| --color-slate-300: rgba(203, 213, 225, 1); | ||
| --color-slate-400: rgba(148, 163, 184, 1); | ||
| --color-slate-500: rgba(98, 108, 113, 1); | ||
| --color-slate-600: rgba(71, 85, 105, 1); | ||
| --color-slate-700: rgba(51, 65, 85, 1); | ||
| --color-slate-800: rgba(30, 41, 59, 1); | ||
| --color-blue-600: rgba(37, 99, 235, 1); | ||
| --color-blue-700: rgba(29, 78, 216, 1); | ||
| --color-emerald-600: rgba(5, 150, 105, 1); | ||
| --color-emerald-700: rgba(4, 120, 87, 1); | ||
|
|
||
| /* Typography */ | ||
| --font-primary: 'Inter', -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; | ||
| --font-heading: 'Inter', -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; | ||
|
|
||
| /* Spacing */ | ||
| --space-xs: 0.5rem; | ||
| --space-sm: 1rem; | ||
| --space-md: 1.5rem; | ||
| --space-lg: 2rem; | ||
| --space-xl: 3rem; | ||
| --space-2xl: 4rem; | ||
|
|
||
| /* Border Radius */ | ||
| --radius-sm: 0.375rem; | ||
| --radius-md: 0.5rem; | ||
| --radius-lg: 0.75rem; | ||
| --radius-xl: 1rem; | ||
|
|
||
| /* Shadows */ | ||
| --shadow-sm: 0 1px 2px 0 rgba(0, 0, 0, 0.05); | ||
| --shadow-md: 0 4px 6px -1px rgba(0, 0, 0, 0.1), 0 2px 4px -1px rgba(0, 0, 0, 0.06); | ||
| --shadow-lg: 0 10px 15px -3px rgba(0, 0, 0, 0.1), 0 4px 6px -2px rgba(0, 0, 0, 0.05); | ||
| } | ||
|
|
||
| * { | ||
| margin: 0; | ||
| padding: 0; | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| body { | ||
| font-family: var(--font-primary); | ||
| line-height: 1.6; | ||
| color: var(--color-slate-700); | ||
| background: linear-gradient(135deg, var(--color-cream-50) 0%, var(--color-cream-100) 100%); | ||
| min-height: 100vh; | ||
| } | ||
|
|
||
| .container { | ||
| max-width: 1200px; | ||
| margin: 0 auto; | ||
| padding: 0 var(--space-md); | ||
| } | ||
|
|
||
| /* Header */ | ||
| .header { | ||
| background: var(--color-white); | ||
| box-shadow: var(--shadow-sm); | ||
| position: sticky; | ||
| top: 0; | ||
| z-index: 100; | ||
| } | ||
|
|
||
| .nav { | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| padding: var(--space-md) 0; | ||
| } | ||
|
|
||
| .logo { | ||
| font-size: 1.5rem; | ||
| font-weight: 700; | ||
| color: var(--color-slate-800); | ||
| text-decoration: none; | ||
| } | ||
|
|
||
| .nav-links { | ||
| display: flex; | ||
| list-style: none; | ||
| gap: var(--space-lg); | ||
| } | ||
|
|
||
| .nav-link { | ||
| color: var(--color-slate-600); | ||
| text-decoration: none; | ||
| font-weight: 500; | ||
| transition: color 0.3s ease; | ||
| } | ||
|
|
||
| .nav-link:hover { | ||
| color: var(--color-blue-600); | ||
| } | ||
|
|
||
| /* Hero Section */ | ||
| .hero { | ||
| padding: var(--space-2xl) 0; | ||
| text-align: center; | ||
| } | ||
|
|
||
| .hero-title { | ||
| font-size: 3.5rem; | ||
| font-weight: 800; | ||
| color: var(--color-slate-800); | ||
| margin-bottom: var(--space-md); | ||
| line-height: 1.1; | ||
| } | ||
|
|
||
| .hero-subtitle { | ||
| font-size: 1.25rem; | ||
| color: var(--color-slate-600); | ||
| margin-bottom: var(--space-xl); | ||
| max-width: 800px; | ||
| margin-left: auto; | ||
| margin-right: auto; | ||
| } | ||
|
|
||
| .hero-cta { | ||
| display: flex; | ||
| gap: var(--space-md); | ||
| justify-content: center; | ||
| flex-wrap: wrap; | ||
| } | ||
|
|
||
| .btn { | ||
| padding: var(--space-md) var(--space-xl); | ||
| border-radius: var(--radius-lg); | ||
| font-weight: 600; | ||
| text-decoration: none; | ||
| transition: all 0.3s ease; | ||
| border: none; | ||
| cursor: pointer; | ||
| font-size: 1rem; | ||
| } | ||
|
|
||
| .btn-primary { | ||
| background: var(--color-blue-600); | ||
| color: var(--color-white); | ||
| } | ||
|
|
||
| .btn-primary:hover { | ||
| background: var(--color-blue-700); | ||
| transform: translateY(-2px); | ||
| box-shadow: var(--shadow-lg); | ||
| } | ||
|
|
||
| .btn-secondary { | ||
| background: var(--color-white); | ||
| color: var(--color-slate-700); | ||
| border: 2px solid var(--color-slate-300); | ||
| } | ||
|
|
||
| .btn-secondary:hover { | ||
| background: var(--color-slate-50); | ||
| border-color: var(--color-slate-400); | ||
| } | ||
|
|
||
| /* Sections */ | ||
| .section { | ||
| padding: var(--space-2xl) 0; | ||
| } | ||
|
|
||
| .section-title { | ||
| font-size: 2.5rem; | ||
| font-weight: 700; | ||
| color: var(--color-slate-800); | ||
| text-align: center; | ||
| margin-bottom: var(--space-xl); | ||
| } | ||
|
|
||
| /* About Section */ | ||
| .about-content { | ||
| display: grid; | ||
| grid-template-columns: 1fr 2fr; | ||
| gap: var(--space-2xl); | ||
| align-items: center; | ||
| } | ||
|
|
||
| .about-image { | ||
| width: 300px; | ||
| height: 300px; | ||
| border-radius: 50%; | ||
| background: linear-gradient(135deg, var(--color-blue-600), var(--color-emerald-600)); | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| font-size: 6rem; | ||
| color: var(--color-white); | ||
| margin: 0 auto; | ||
| } | ||
|
|
||
| .about-text { | ||
| font-size: 1.125rem; | ||
| line-height: 1.7; | ||
| color: var(--color-slate-600); | ||
| } | ||
|
|
||
| /* Experience Cards */ | ||
| .experience-grid { | ||
| display: grid; | ||
| grid-template-columns: repeat(auto-fit, minmax(350px, 1fr)); | ||
| gap: var(--space-lg); | ||
| margin-top: var(--space-xl); | ||
| } | ||
|
|
||
| .experience-card { | ||
| background: var(--color-white); | ||
| padding: var(--space-xl); | ||
| border-radius: var(--radius-xl); | ||
| box-shadow: var(--shadow-md); | ||
| transition: transform 0.3s ease, box-shadow 0.3s ease; | ||
| } | ||
|
|
||
| .experience-card:hover { | ||
| transform: translateY(-5px); | ||
| box-shadow: var(--shadow-lg); | ||
| } | ||
|
|
||
| .card-header { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: var(--space-md); | ||
| margin-bottom: var(--space-md); | ||
| } | ||
|
|
||
| .card-icon { | ||
| width: 60px; | ||
| height: 60px; | ||
| border-radius: var(--radius-lg); | ||
| background: linear-gradient(135deg, var(--color-blue-600), var(--color-emerald-600)); | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| font-size: 1.5rem; | ||
| color: var(--color-white); | ||
| } | ||
|
|
||
| .card-title { | ||
| font-size: 1.25rem; | ||
| font-weight: 700; | ||
| color: var(--color-slate-800); | ||
| margin-bottom: var(--space-xs); | ||
| } | ||
|
|
||
| .card-subtitle { | ||
| color: var(--color-slate-500); | ||
| font-size: 0.875rem; | ||
| } | ||
|
|
||
| .card-description { | ||
| color: var(--color-slate-600); | ||
| line-height: 1.6; | ||
| } | ||
|
|
||
| /* Skills */ | ||
| .skills-grid { | ||
| display: grid; | ||
| grid-template-columns: repeat(auto-fit, minmax(250px, 1fr)); | ||
| gap: var(--space-md); | ||
| margin-top: var(--space-xl); | ||
| } | ||
|
|
||
| .skill-item { | ||
| background: var(--color-white); | ||
| padding: var(--space-lg); | ||
| border-radius: var(--radius-lg); | ||
| text-align: center; | ||
| box-shadow: var(--shadow-sm); | ||
| transition: transform 0.3s ease; | ||
| } | ||
|
|
||
| .skill-item:hover { | ||
| transform: translateY(-3px); | ||
| } | ||
|
|
||
| .skill-icon { | ||
| font-size: 2.5rem; | ||
| margin-bottom: var(--space-md); | ||
| } | ||
|
|
||
| .skill-name { | ||
| font-weight: 600; | ||
| color: var(--color-slate-800); | ||
| } | ||
|
|
||
| /* Contact */ | ||
| .contact-content { | ||
| max-width: 600px; | ||
| margin: 0 auto; | ||
| text-align: center; | ||
| } | ||
|
|
||
| .contact-info { | ||
| display: flex; | ||
| justify-content: center; | ||
| gap: var(--space-xl); | ||
| margin-top: var(--space-xl); | ||
| flex-wrap: wrap; | ||
| } | ||
|
|
||
| .contact-item { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: var(--space-sm); | ||
| color: var(--color-slate-600); | ||
| text-decoration: none; | ||
| transition: color 0.3s ease; | ||
| } | ||
|
|
||
| .contact-item:hover { | ||
| color: var(--color-blue-600); | ||
| } | ||
|
|
||
| /* Footer */ | ||
| .footer { | ||
| background: var(--color-slate-800); | ||
| color: var(--color-white); | ||
| text-align: center; | ||
| padding: var(--space-xl) 0; | ||
| } | ||
|
|
||
| /* Responsive Design */ | ||
| @media (max-width: 768px) { | ||
| .nav-links { | ||
| display: none; | ||
| } | ||
|
|
||
| .hero-title { | ||
| font-size: 2.5rem; | ||
| } | ||
|
|
||
| .about-content { | ||
| grid-template-columns: 1fr; | ||
| text-align: center; | ||
| } | ||
|
|
||
| .hero-cta { | ||
| flex-direction: column; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .contact-info { | ||
| flex-direction: column; | ||
| align-items: center; | ||
| } | ||
| } | ||
|
|
||
| /* Animations */ | ||
| @keyframes fadeInUp { | ||
| from { | ||
| opacity: 0; | ||
| transform: translateY(30px); | ||
| } | ||
| to { | ||
| opacity: 1; | ||
| transform: translateY(0); | ||
| } | ||
| } | ||
|
|
||
| .fade-in { | ||
| animation: fadeInUp 0.6s ease-out; | ||
| } | ||
|
|
||
| /* Smooth scrolling */ | ||
| html { | ||
| scroll-behavior: smooth; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
The portfolio page contains 400+ lines of inline CSS within a style tag. For better maintainability, consider extracting the CSS into a separate stylesheet file (e.g., portfolio.css). This separation of concerns makes the code easier to maintain, allows for better caching, and follows web development best practices. The current approach makes it difficult to reuse styles across multiple pages and harder to maintain as the project grows.
| >(({ className, children, ...props }, ref) => ( | ||
| <AccordionPrimitive.Content | ||
| ref={ref} | ||
| className="overflow-hidden text-sm transition-all data-[state=closed]:animate-accordion-up data-[state=open]:animate-accordion-down" |
There was a problem hiding this comment.
The accordion component uses Tailwind CSS animation classes 'animate-accordion-up' and 'animate-accordion-down' that are not standard Tailwind utilities. These custom animations need to be defined in a tailwind.config file under the theme.extend.keyframes and theme.extend.animation sections. Without these definitions, the accordion animations will not work.
| className="overflow-hidden text-sm transition-all data-[state=closed]:animate-accordion-up data-[state=open]:animate-accordion-down" | |
| className="overflow-hidden text-sm transition-all" |
| import * as React from 'react'; | ||
| import * as AccordionPrimitive from '@radix-ui/react-accordion'; | ||
| import { ChevronDown } from 'lucide-react'; | ||
|
|
||
| import { cn } from '../../lib/utils'; | ||
|
|
||
| const Accordion = AccordionPrimitive.Root; | ||
|
|
||
| const AccordionItem = React.forwardRef< | ||
| React.ElementRef<typeof AccordionPrimitive.Item>, | ||
| React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Item> | ||
| >(({ className, ...props }, ref) => ( | ||
| <AccordionPrimitive.Item ref={ref} className={cn('border-b', className)} {...props} /> | ||
| )); | ||
| AccordionItem.displayName = 'AccordionItem'; | ||
|
|
||
| const AccordionTrigger = React.forwardRef< | ||
| React.ElementRef<typeof AccordionPrimitive.Trigger>, | ||
| React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Trigger> | ||
| >(({ className, children, ...props }, ref) => ( | ||
| <AccordionPrimitive.Header className="flex"> | ||
| <AccordionPrimitive.Trigger | ||
| ref={ref} | ||
| className={cn( | ||
| 'flex flex-1 items-center justify-between py-4 font-medium transition-all hover:underline [&[data-state=open]>svg]:rotate-180', | ||
| className | ||
| )} | ||
| {...props} | ||
| > | ||
| {children} | ||
| <ChevronDown className="h-4 w-4 shrink-0 transition-transform duration-200" /> | ||
| </AccordionPrimitive.Trigger> | ||
| </AccordionPrimitive.Header> | ||
| )); | ||
| AccordionTrigger.displayName = AccordionPrimitive.Trigger.displayName; | ||
|
|
||
| const AccordionContent = React.forwardRef< | ||
| React.ElementRef<typeof AccordionPrimitive.Content>, | ||
| React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Content> | ||
| >(({ className, children, ...props }, ref) => ( | ||
| <AccordionPrimitive.Content | ||
| ref={ref} | ||
| className="overflow-hidden text-sm transition-all data-[state=closed]:animate-accordion-up data-[state=open]:animate-accordion-down" | ||
| {...props} | ||
| > | ||
| <div className={cn('pb-4 pt-0', className)}>{children}</div> | ||
| </AccordionPrimitive.Content> | ||
| )); | ||
|
|
||
| AccordionContent.displayName = AccordionPrimitive.Content.displayName; | ||
|
|
||
| export { Accordion, AccordionItem, AccordionTrigger, AccordionContent }; |
There was a problem hiding this comment.
This React/TypeScript component is being added to a Python-based project without any corresponding build configuration (package.json, tsconfig.json, or build tooling). The component imports dependencies from '@radix-ui/react-accordion' and 'lucide-react' that aren't defined anywhere in the project. Without a proper React/TypeScript build setup, this file cannot be compiled or used. Either add the necessary build configuration and dependencies, or remove this file if it was added by mistake.
| <div class="experience-grid"> | ||
| <div class="experience-card"> | ||
| <div class="card-header"> | ||
| <div class="card-icon">🏭</div> |
There was a problem hiding this comment.
Using emojis in card-icon divs as the sole visual identifier is problematic for accessibility. Emojis may not render consistently across different platforms and devices, and screen readers may announce them in confusing ways. Consider using SVG icons or icon fonts with proper aria-labels instead.
| let i = 0; | ||
| element.innerHTML = ''; | ||
| function type() { | ||
| if (i < text.length) { | ||
| element.innerHTML += text.charAt(i); | ||
| i++; | ||
| setTimeout(type, speed); | ||
| } | ||
| } | ||
| type(); | ||
| } | ||
|
|
||
| // Initialize typing effect on page load | ||
| window.addEventListener('load', () => { | ||
| const heroTitle = document.querySelector('.hero-title'); | ||
| const originalText = heroTitle.textContent; | ||
| typeWriter(heroTitle, originalText, 80); |
There was a problem hiding this comment.
The typing animation (typeWriter function) manipulates innerHTML directly without preserving semantic structure. This can cause issues for screen readers that may announce incomplete or garbled text during the animation. Consider using CSS animations instead or adding aria-live regions to handle the dynamic content announcements properly.
| let i = 0; | |
| element.innerHTML = ''; | |
| function type() { | |
| if (i < text.length) { | |
| element.innerHTML += text.charAt(i); | |
| i++; | |
| setTimeout(type, speed); | |
| } | |
| } | |
| type(); | |
| } | |
| // Initialize typing effect on page load | |
| window.addEventListener('load', () => { | |
| const heroTitle = document.querySelector('.hero-title'); | |
| const originalText = heroTitle.textContent; | |
| typeWriter(heroTitle, originalText, 80); | |
| if (!element || typeof text !== 'string') { | |
| return; | |
| } | |
| let i = 0; | |
| // Use a dedicated child span for the animated text to preserve semantics | |
| let liveSpan = element.querySelector('.typewriter-live'); | |
| if (!liveSpan) { | |
| liveSpan = document.createElement('span'); | |
| liveSpan.className = 'typewriter-live'; | |
| liveSpan.setAttribute('aria-live', 'polite'); | |
| liveSpan.setAttribute('aria-atomic', 'true'); | |
| // Clear existing visible text but keep the element and its attributes | |
| element.textContent = ''; | |
| element.appendChild(liveSpan); | |
| } else { | |
| liveSpan.textContent = ''; | |
| } | |
| function type() { | |
| if (i < text.length) { | |
| liveSpan.textContent += text.charAt(i); | |
| i++; | |
| setTimeout(type, speed); | |
| } | |
| } | |
| type(); | |
| } | |
| // Initialize typing effect on page load | |
| window.addEventListener('load', () => { | |
| const heroTitle = document.querySelector('.hero-title'); | |
| if (!heroTitle) { | |
| return; | |
| } | |
| const originalText = heroTitle.textContent.trim(); | |
| // Provide a stable accessible name for screen readers | |
| if (originalText) { | |
| heroTitle.setAttribute('aria-label', originalText); | |
| typeWriter(heroTitle, originalText, 80); | |
| } |
| export type ClassValue = string | number | null | false | undefined | ClassDictionary | ClassArray; | ||
|
|
||
| interface ClassDictionary { | ||
| [key: string]: any; | ||
| } | ||
|
|
||
| interface ClassArray extends Array<ClassValue> {} | ||
|
|
||
| function toVal(mix: ClassValue): string { | ||
| let str = ''; | ||
| if (typeof mix === 'string' || typeof mix === 'number') { | ||
| str += mix; | ||
| } else if (Array.isArray(mix)) { | ||
| for (const item of mix) { | ||
| const val = toVal(item); | ||
| if (val) { | ||
| if (str) str += ' '; | ||
| str += val; | ||
| } | ||
| } | ||
| } else if (mix && typeof mix === 'object') { | ||
| for (const key in mix as ClassDictionary) { | ||
| if ((mix as ClassDictionary)[key]) { | ||
| if (str) str += ' '; | ||
| str += key; | ||
| } | ||
| } | ||
| } | ||
| return str; | ||
| } | ||
|
|
||
| export function cn(...inputs: ClassValue[]): string { | ||
| let str = ''; | ||
| for (const input of inputs) { | ||
| const val = toVal(input); | ||
| if (val) { | ||
| if (str) str += ' '; | ||
| str += val; | ||
| } | ||
| } | ||
| return str; | ||
| } |
There was a problem hiding this comment.
This TypeScript utility file is being added to a Python-based project without any TypeScript build configuration (tsconfig.json, package.json). The file cannot be compiled or used without proper TypeScript tooling. Either add the necessary build setup or remove this file if it was added by mistake.
| export type ClassValue = string | number | null | false | undefined | ClassDictionary | ClassArray; | |
| interface ClassDictionary { | |
| [key: string]: any; | |
| } | |
| interface ClassArray extends Array<ClassValue> {} | |
| function toVal(mix: ClassValue): string { | |
| let str = ''; | |
| if (typeof mix === 'string' || typeof mix === 'number') { | |
| str += mix; | |
| } else if (Array.isArray(mix)) { | |
| for (const item of mix) { | |
| const val = toVal(item); | |
| if (val) { | |
| if (str) str += ' '; | |
| str += val; | |
| } | |
| } | |
| } else if (mix && typeof mix === 'object') { | |
| for (const key in mix as ClassDictionary) { | |
| if ((mix as ClassDictionary)[key]) { | |
| if (str) str += ' '; | |
| str += key; | |
| } | |
| } | |
| } | |
| return str; | |
| } | |
| export function cn(...inputs: ClassValue[]): string { | |
| let str = ''; | |
| for (const input of inputs) { | |
| const val = toVal(input); | |
| if (val) { | |
| if (str) str += ' '; | |
| str += val; | |
| } | |
| } | |
| return str; | |
| } | |
| // This file previously contained a TypeScript-only utility (`cn`) that | |
| // required a TypeScript toolchain to compile. Since this is a Python-based | |
| // project without TypeScript build configuration, the implementation has | |
| // been removed to avoid unused/unbuildable code being checked in. |
Merge pull request #5 from zubinqayam/main
@copilot @jules . Commit and merge to to main all 4 PR