Skip to content

Refactor/breakup Nav component #2224

Closed
@lindapaiste

Description

@lindapaiste

Increasing Access

  • It will help @dewanshDT in his GSoC project...
  • ... which will help make the editor mobile responsive and more accessible to everyone!
  • It will also help future contributors who work on this component.

Feature enhancement details

The Nav component is over 1,000 lines long and has a lot going on. I want @dewanshDT to start working on making this UI mobile-friendly, but the component is very unwieldy. I would like to refactor some of the logic first and then he will have an easier time working on the styling.

Breakup

I suggest that we create individual components for various parts of the UI, such as NavBar, DropdownMenu, MenuItem, etc.

Defining Lists

We can define the contents of the menu as a tree of JSX elements, like we are doing now. That would look something like this:

<NavBar>
  <DropdownMenu key="file" title="File">
    <MenuItem onClick={handleNew}>New</MenuItem>
    <MenuItem href="/p5/sketches">Examples</MenuItem>
  </DropdownMenu>
</NavBar>

We could also use a structure of JavaScript objects and arrays. This was suggested back in 2019 and is the way that it's handled in the unused MobileIDEView. That would look like:

const navLinks = [
  {
    key: 'file',
    title: 'File',
    subMenu: [
      {
        key: 'new',
        title: 'New',
        onClick: handleNew
      },
      {
        key: 'examples',
        title: 'Examples',
        href: '/p5/sketches'
      }
    ]
  }
];

My personal opinion is that the JSX is better as it's more contributor-friendly. Right now the drawback of the JSX is that we are passing a lot of the same props to each element, but that can be cleaned up.

Event Handling

We are passing down onBlur, onFocus, and onClick handlers to each item in the dropdown menus. My goal is to get rid of these props but be still be able to trigger changes in menu state from child components.

I'm thinking that React Contexts is probably the best way, though injecting props with React.Children.map() and React.cloneElement() is also an option.

A MenuItem would be aware of which DropdownMenu is its parent. It would have access to handler functions from some sort of context provided by the parent NavBar. That way the MenuItem can deal with the event handling internally and the component itself doesn't need to take a lot of props.

Metadata

Metadata

Assignees

No one assigned

    Labels

    EnhancementImprovement to an existing feature

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions