accordproject / template-playground

Web Playground for TemplateMark templates
https://playground.accordproject.org
Apache License 2.0
18 stars 22 forks source link

Refactor Menu Links Using Constants and Map Function for Better Code Structure as per industry standards in src/components/Navbar.tsx #107

Open yashpandey06 opened 2 months ago

yashpandey06 commented 2 months ago

Title: Refactor Menu Links Using Constants and Map Function for Better Code Structure

Description:
In the current implementation of the Menu.ItemGroup, the href, target, rel, and other fields are hardcoded directly in the JSX. This approach can lead to redundancy and reduce maintainability, especially as the application scales.

Current Scenerio : WhatsApp Image 2024-09-23 at 11 23 36

Proposed Solution:
To improve the code quality and follow industry-level best practices, we can:

  1. Store the links (href, target, rel, etc.) in a constant file to centralize these values and make them easier to manage.
  2. Use the map() function to dynamically render Menu.ItemGroup components. This will not only reduce code duplication but also make it more maintainable and scalable.

Benefits:

Example:


const menuItems = [
  { key: 'about', href: 'https://github.com/accordproject/template-playground/blob/main/README.md', icon: <QuestionOutlined />, label: 'About' },
  { key: 'community', href: 'https://discord.com/invite/Zm99SKhhtA', icon: <UserOutlined />, label: 'Community' },
  { key: 'issues', href: 'https://github.com/accordproject/template-playground/issues', icon: <InfoOutlined />, label: 'Issues' },
  { key: 'documentation', href: 'https://github.com/accordproject/template-engine/blob/main/README.md', icon: <BookOutlined />, label: 'Documentation' },
];

return (
  <Menu.ItemGroup title="Info">
    {menuItems.map(item => (
      <Menu.Item key={item.key}>
        <a href={item.href} target="_blank" rel="noopener noreferrer">
          {item.icon} {item.label}
        </a>
      </Menu.Item>
    ))}
  </Menu.ItemGroup>
);
Vinyl-Davyl commented 2 months ago

Hey @yashpandey06 πŸ‘‹ Great point out. Really a valid one. I agree!

Do you have intentions with this? If not, surely we would look into this, definitely.

yashpandey06 commented 2 months ago

Hey @yashpandey06 πŸ‘‹ Great point out. Really a valid one. I agree!

Do you have intentions with this? If not, surely we would look into this, definitely.

Yeah , I would like to work on this ☺️.If you have not started yet.

Vinyl-Davyl commented 2 months ago

Okay, would leave that to you. Sure you can! @yashpandey06

sure be mindful of the test cases in place for the active links in the tests folder. should incase there are any breaks in the test while at it to be modified.

yashpandey06 commented 2 months ago

I'm not sure why, but when I try to start my Vite server, the screen is completely blank with no errors being thrown. Could you help me with this?

@Vinyl-Davyl

Vinyl-Davyl commented 2 months ago

I'm not sure why, but when I try to start my Vite server, the screen is completely blank with no errors being thrown. Could you help me with this?

@Vinyl-Davyl

Hey Yash, I guess you should check your console on the browser, you would probably see something relating to the <Router> or BrowserRouter. So as a fresh start to the application, vite is probably loading up the Mixed ESM and CJS like in the @accordproject/template-engine and more. So it's a cold restart.

I suggest you give it some more time, try clearing your cache or deleting and install your node modules again if possibe npm cache clean --force rm -rf node_modules npm install. Let me know when you make progress with this