Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: created the developer nav bar #707

Open
wants to merge 3 commits into
base: io/developer-dashboard
Choose a base branch
from

Conversation

Ndohjapan
Copy link
Contributor

Describe your changes

Created the Navbar for developer dashbaord

Issue ticket number and link

#702

Screenshots (if appropriate):

image
image

Copy link
Collaborator

@ijemmao ijemmao left a comment

Choose a reason for hiding this comment

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

great start~

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Ndohjapan please delete this file

.yarnrc Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Ndohjapan please delete this file

Comment on lines 2 to 3
import { Dialog } from '@headlessui/react';
import { Bars3Icon, XMarkIcon } from '@heroicons/react/24/outline';
Copy link
Collaborator

Choose a reason for hiding this comment

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

we don't need these packages since we have icons from material UI or chakra - please uninstall these packages

const [mobileMenuOpen, setMobileMenuOpen] = useState(false);

return (
<div className="bg-white">
Copy link
Collaborator

Choose a reason for hiding this comment

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

we use ChakraUI for this project - please replace all div, a, h1, etc. with their ChakraUI equivalents (Box, Link, Heading, etc.)


return (
<div className="bg-white">
<header className="absolute inset-x-0 top-0 z-50">
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's not typical to use the header tag - please refer to the navigation bar that's used on the homepage for inspiration

];

export default function NewHeader() {
const [mobileMenuOpen, setMobileMenuOpen] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const [mobileMenuOpen, setMobileMenuOpen] = useState(false);
const [isMobileMenuOpen, setIsMobileMenuOpen] = useState(false);

@@ -0,0 +1,81 @@
import {
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: why are you creating a brand new navbar? these are the same options that we have in our existing navbar - couldn't we reuse the navbar that we already have?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually wanted to use the existing nav bar.

However the Figma design is a bit different from it and I thought it was intentional from the design to make that of the developer dashboard different from the main nav bar.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, let's confirm with David if we can update the Figma designs so they match our current navbar, or if he wants to update our existing navbar to match his newer designs

i think having two navbars at right now might be a bit much for us to maintain!

@ijemmao ijemmao force-pushed the io/developer-dashboard branch 6 times, most recently from fb6ed0f to 1f29c6c Compare March 24, 2024 02:00
@ijemmao ijemmao force-pushed the io/developer-dashboard branch 2 times, most recently from c9ef97f to 8d08544 Compare May 5, 2024 21:14
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.

None yet

2 participants