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

➡️ 1027 navbar top right buttons #1034

Merged
merged 12 commits into from
Jul 13, 2023
Merged

Conversation

jasangill1
Copy link
Contributor

##About

  • Used the primary buttons as a template,

  • Font styling not apply to button wondering if this was the correct approach

  • Still working on screen alignment for nav layout, Looking for review.

image

## About

- Basic template wanting inputs

- Used primary button as template

- Need to fix text sizing
@jasangill1 jasangill1 linked an issue Jul 13, 2023 that may be closed by this pull request
@vercel
Copy link

vercel bot commented Jul 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
agent-gpt ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 3:38am

@ergomake
Copy link

ergomake bot commented Jul 13, 2023

Hi 👋

Here's a preview environment 🚀

https://next-reworkd-agentgpt-1034.env.ergomake.link

Environment Summary 📑

Container Source URL
next Dockerfile https://next-reworkd-agentgpt-1034.env.ergomake.link
platform Dockerfile https://platform-reworkd-agentgpt-1034.env.ergomake.link
db Dockerfile [not exposed - internal service]
weaviate semitechnologies/weaviate:1.19.6 https://weaviate-reworkd-agentgpt-1034.env.ergomake.link

Here are your environment's logs

For questions or comments, join Discord.

Click here to disable Ergomake.

- Made the width the same as the footer the right side is aligned but the the left isnt
<Disclosure as="nav" className="absolute top-0 z-50 w-full bg-transparent text-white">
<Disclosure
as="nav"
className="hidden:overlay absolute top-0 z-50 w-full bg-transparent text-white"
Copy link
Member

Choose a reason for hiding this comment

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

Ideally we want to be using absolute as less as possible as this will cause problems down the road with small screens. I think we can do the header - content - footer all with flex

Comment on lines 20 to 21
{icon}
{children}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should just make this children and pass everything in for max configurability :)

@@ -7,7 +7,7 @@ const HomePage = () => {
<NavLayout>
<div className="flex w-full justify-center">
<div className="flex max-w-screen-2xl flex-col items-center justify-center overflow-x-hidden px-5 text-white">
<div className="flex h-screen w-full flex-col items-start justify-center overflow-x-hidden px-4 text-white lg:pl-16">
Copy link
Contributor

Choose a reason for hiding this comment

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

lets keep this. Good to have some padding

Comment on lines 1 to 24
import clsx from "clsx";
import Button from "../ui/button";
import type { ReactNode } from "react";
import React from "react";

type PrimaryButtonProps = {
children: ReactNode | string;
icon?: React.ReactNode;
onClick?: () => void;
};
export default function NavbarContactButton({ children, onClick, icon }: PrimaryButtonProps) {
return (
<Button
onClick={onClick}
className={clsx(
"flex h-8 items-center justify-center rounded-full pl-2 shadow-sm",
"ml-4 transition duration-200 ease-in-out hover:hover:bg-white/80",
"bg-white text-black"
)}
>
{children}
</Button>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be reusing the same Button as the hero text. Why do we need to make custom ones?

<Menu as="div" className="relative ml-3">
<div></div>
</Menu>
<div className="ml-auto hidden sm:ml-6 sm:flex sm:items-center">
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need the ml-auto?

@asim-shrestha asim-shrestha merged commit 577aaa1 into main Jul 13, 2023
@awtkns awtkns deleted the 1027-navbar-top-right-buttons branch August 3, 2023 20:44
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.

🧭 NavBar Top Right Buttons
3 participants