Skip to content

Update: committed latest changes#401

Open
Gupta-02 wants to merge 2 commits intoRoshansuthar1105:mainfrom
Gupta-02:main
Open

Update: committed latest changes#401
Gupta-02 wants to merge 2 commits intoRoshansuthar1105:mainfrom
Gupta-02:main

Conversation

@Gupta-02
Copy link

UI / UX

  • Implemented a professional desktop navigation bar (always-visible links on xl screens; condensed set on lg screens; hamburger retained for mobile).
  • Extracted and modularized Hero section into Hero.jsx for maintainability and eliminated previous layout shift / button “jumping”.
  • Added accessible focus-visible ring styles to all nav links (desktop + mobile) and CTA buttons.
  • Unified navigation link definitions via NAV_LINKS and CONDENSED_LINKS constants (navLinks.js) to remove duplication between NavBar.jsx and MobileMenu.jsx.

Code Quality / Structure

  • Refactored Home.jsx to use <Hero /> component.
  • Reduced repeated nav markup and improved long-term maintainability.
  • Added Playwright end-to-end test setup (@playwright/test, playwright.config.js, tests/navbar.spec.js).
  • Updated README.md with accessibility + E2E testing instructions.
  • Added PR template (if not already present) / verified existing template.

Tooling / Scripts

  • Added E2E test scripts: test:e2e, test:e2e:ui, test:e2e:headed to package.json.

🧪 Testing

  • Client build (Vite) completed successfully (no compile errors).
  • Manual UI verification:
    • Desktop (≥1400px): All nav links visible, hamburger hidden.
    • Large (lg < xl): Condensed subset visible.
    • Mobile (≤480px): Hamburger visible; links appear after opening mobile menu.
    • Hero layout stable (no vertical jumps on load).
    • Keyboard navigation: Tab focus shows ring around links & CTAs.
  • Playwright test added (tests/navbar.spec.js) covering:
    • Full nav presence on desktop.
    • Hamburger + dynamic menu on mobile.
  • (Optional—run later) Server dependency install succeeded; runtime start not included in this PR scope.

📸 UI Changes (Summary)

(If you have screenshots, add them)

Before | After -- | -- Nav links hidden behind hamburger on large monitors | Full professional navbar visible on desktop Inline hero markup in Home.jsx | Modular Hero.jsx component No consistent focus ring | Accessible focus-visible rings

Documentation Updates

  •  Updated README.md
  •  Added explanation of Playwright usage
  •  Mentioned accessibility improvements

  •  New branch created for changes
  •  No console errors during manual testing
  •  Commit messages follow guidelines
  •  Accessibility pass (keyboard focus)
  •  E2E test scaffold added
  •  (Optional) Linked issue created
  •  (Optional) Added screenshots

Notable Notes / Follow-Up Suggestions

  • Bundle size warnings: Large chunks (e.g. Code Editor + large note modules). Future improvement: dynamic splitting.
  • Consider adding .env.example and adjusting server "start" script to use node server.js (currently nodemon in production).
  • Optional enhancement: Add CI workflow to run npm run build + npm run test:e2e.
  • Add tests for mobile menu close-on-route-change and admin-only links (future).

Additional Notes

No breaking changes. Safe to deploy after visual regression review.


If you want, I can also generate suggested commit messages or a squash message—just say the word. Want me to proceed with the server runtime sanity check next or include an .env.example? Let me know.

UI / UX

@netlify
Copy link

netlify bot commented Sep 15, 2025

Deploy Preview for codifylearn ready!

Name Link
🔨 Latest commit 7ab71c5
🔍 Latest deploy log https://app.netlify.com/projects/codifylearn/deploys/68c7870e38be0000080e7ca7
😎 Deploy Preview https://deploy-preview-401--codifylearn.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@Gupta-02
Copy link
Author

@Roshansuthar1105 Please check the pr sir

@Roshansuthar1105
Copy link
Owner

@Gupta-02 resolve the conflicts please.

@Gupta-02
Copy link
Author

Okay wait

import React from 'react';
import { motion } from 'framer-motion';
import { useTheme } from '../context/ThemeContext';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reduce motion for accessibility

Respect prefers-reduced-motion for Framer Motion animations to improve accessibility.

Suggestion

const prefersReducedMotion = useReducedMotion();

@@ -72,46 +73,48 @@ function NavBar() {

</div>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move animation configs (initial, animate, transition) into reusable constants.

Use a shared Button component for CTA buttons to avoid duplication.

Add PropTypes or TypeScript for components like MobileMenu.

// Each link: { to: string, label: string, icon: ReactIconComponent, group?: 'primary' | 'secondary' }
// Order matters for display.
import { FaHome, FaUser, FaCode, FaBookOpen, FaRoad, FaStickyNote, FaQuestionCircle, FaBookmark, FaUserFriends, FaEnvelope } from 'react-icons/fa';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract repeated Tailwind classes into reusable utility classes.

Replace magic values like pt-32 sm:pt-40 with CSS variables linked to navbar height.

Ensure min-h-screen-minus-nav is documented in global styles.

@@ -16,6 +16,7 @@ const Testimonials = lazy(() => import("../components/HomePageComponents/Testimo
const NewsLetter = lazy(() => import("../components/HomePageComponents/NewsLetter"));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add aria-current="page" to active NavLinks.

Add aria-label to CTA buttons like “Watch Demo”.

Ensure video has aria-hidden="true" if it’s decorative.

@netlify
Copy link

netlify bot commented Feb 5, 2026

👷 Deploy request for codifylearn pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 86af747

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants