Refactor Nav pt. 2 - create sub-components #2227
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2224
Extends PR #2210
Changes:
This PR should not change any UI or behavior, with one tiny exception: I removed a few
title=
tags which were causing unhelpful tooltips (see also #2150, #1266).I'm changing how the UI is generated by breaking it up into various building blocks that can be put together to form the current
Nav
. I'm cutting pieces of code out ofNav
and pasting them into new files. In the best cases, I'm cutting code that's repeated in multiple spots and pasting it only to one usage.New Components
NavBar
The
NavBar
is the parent that wraps all of the elements in the navigation.Nav
.Esc
and close on outside click behavior.header
andnav
elements.NavDropdownMenu
The
NavDropdownMenu
is a top-level item in the nav bar which contains a submenu (File, Edit, etc.).id
to its children via context.NavMenuItem
The
NavMenuItem
is for each item a dropdown menu.li
with the correctclassName
(likely to be changed to a styled component in the future).hideIf
for cleaner conditional rendering.ButtonOrLink
.ButtonOrLink
I put this in the
common
folder because it can be used in other places.ButtonOrLink
is a helper that allows buttons and links to be used interchangeably.button
when there is nohref
prop.href
is an external link, it renders ana
and automatically addstarget="_blank" rel="noopener noreferrer"
.href
is an internal link, it renders areact-router
Link
.Changes to
Nav
and replace with:
and replace with:
NavBar
.dropdownOpen
state intoNavBar
, along with associated logic fromtoggleDropdown
,handleFocus
,handleBlur
,clearHideTimeout
, etc.this.setDropdown('none');
at the end of eachhandle
function because this is now handled by theNavMenuItem
(on theonMouseUp
event, so that it doesn't conflict withonClick
props).onClick={this.props.stopSketch}
instead ofonClick={this.handleStop}
.The
Nav
component can continue to be broken up and cleaned up. It's easy to convert it to a function component now because all of the logic has been moved elsewhere. But I wanted to put in this PR while the general structure ofNav
is the same so that it's easier to see and review the changes.I have verified that this pull request:
npm run lint
)npm run test
)develop
branch.Fixes #123