Skip to content

fix #259 partially converting some of the components from headlessui to shadcn #531

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

Merged
merged 11 commits into from
Jul 31, 2024

Conversation

doppedheart
Copy link
Contributor

fix #259
change the faq from disclore to accordian
Screenshot 2024-07-22 203158
and also the dropdownproto
Screenshot 2024-07-22 203153
Screenshot 2024-07-22 203126

@doppedheart doppedheart requested a review from mfts as a code owner July 22, 2024 15:06
Copy link

vercel bot commented Jul 22, 2024

@doppedheart is attempting to deploy a commit to the mftsio Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Owner

Choose a reason for hiding this comment

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

This should be a select component from shadcn not a popover.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay will make the changes

import { Fragment, Dispatch, SetStateAction } from "react";
import { Dispatch, SetStateAction, useState } from "react";
import { PopoverContent } from "@radix-ui/react-popover";
import { PopoverTrigger, Popover } from "@/components/ui/popover";

function classNames(...classes: string[]) {
Copy link
Owner

Choose a reason for hiding this comment

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

If you want you can replace this with cn from @/lib/utils. it's a better way to merge tailwind classes then this helper function.

@doppedheart
Copy link
Contributor Author

there is same code written in two different files
DropdownProto.tsx -> app/(static)/deck/components/DropdownProto.tsx
dropdownpropt.tsx -> components/web/alternatives/dropdownproto.tsx

should i make the same changes as removing the headlessui components with shadcn in both
or remove any one file and make the corresponding file import changes related to other

@doppedheart
Copy link
Contributor Author

other than these ui changes there are two more files which contain headlessui ,

  1. pages/ai.tsx
  2. pages/share-notion-page.tsx
    both contain the same Faq component (disclosure as dependency) should i import the component over there as that would be much more cleaner approch,
    or else do the same as Faq component but it would be code repetition

@mfts
Copy link
Owner

mfts commented Jul 26, 2024

  1. pages/ai.tsx
  2. pages/share-notion-page.tsx
    both contain the same Faq component (disclosure as dependency) should i import the component over there as that would be much more cleaner approch,

yes that would be preferred! Thank you for spotting that and suggesting this change :)

Please also merge in the current main branch so that merging becomes easier.

@doppedheart doppedheart requested a review from mfts July 26, 2024 21:36
@doppedheart
Copy link
Contributor Author

made changes to all the files and completely removed headlessui
Screenshot 2024-07-27 025342
Screenshot 2024-07-27 025016
Screenshot 2024-07-27 025418

Comment on lines 23 to 39
const [menuOpen, setMenuOpen] = useState<boolean>(false);
const [isFirstClick, setIsFirstClick] = useState<boolean>(false);

<Transition
as={Fragment}
enter="transition ease-out duration-100"
enterFrom="transform opacity-0 scale-95"
enterTo="transform opacity-100 scale-100"
leave="transition ease-in duration-75"
leaveFrom="transform opacity-100 scale-100"
leaveTo="transform opacity-0 scale-95"
>
<Menu.Items
className="absolute left-0 z-10 mt-2 w-full origin-top-right rounded-md bg-white shadow-lg ring-1 ring-black ring-opacity-5 focus:outline-none"
key={option}
>
<div className="">
{options.map((optionItem) => (
<Menu.Item key={optionItem}>
{({ active }) => (
<button
onClick={() => setOption(optionItem)}
className={classNames(
active ? "bg-gray-100 text-gray-900" : "text-gray-700",
option === optionItem ? "bg-gray-200" : "",
"px-4 py-2 text-sm w-full text-left flex items-center space-x-2 justify-between",
)}
>
<span>{optionItem}</span>
{option === optionItem ? (
<CheckIcon className="w-4 h-4 text-bold" />
) : null}
</button>
)}
</Menu.Item>
))}
</div>
</Menu.Items>
</Transition>
</Menu>
);
const handleMenuStateChange = (open: boolean) => {
if (isFirstClick) {
setMenuOpen(true); // Keep the dropdown open on the first click
return;
}

// If the menu is closed, reset the isFirstClick state
if (!open) {
setIsFirstClick(false);
setMenuOpen(false); // Ensure the dropdown is closed
} else {
setMenuOpen(true); // Open the dropdown
}
};
Copy link
Owner

Choose a reason for hiding this comment

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

This is not needed. This is only to for a double-click delete confirmation

}
};
return (
<Select open={menuOpen} onOpenChange={handleMenuStateChange}>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<Select open={menuOpen} onOpenChange={handleMenuStateChange}>
<Select>

Copy link
Owner

Choose a reason for hiding this comment

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

No need to change the accordion behaviour. Let's leave the chevron. This component is used in other places and relies on the chevron.

pages/ai.tsx Outdated
Comment on lines 70 to 113
const faqs = [
{
question: "What is Papermark AI?",
answer:
"Papermark AI is an innovative AI document assistant that enables users to interact with a variety of documents, such as pitch decks, sales decks, and PDFs, in a more efficient and secure manner.",
},
{
question: "How can I use Papermark AI?",
answer:
"You can use it on shared with you document and on received. You can chat with the document, ask question, find information without checking whole document.",
},
{
question: "Is Papermark AI free?",
answer:
"Yes, Papermark AI offers an open-source version, giving you the freedom to use and modify it according to your needs, under the terms of our license.",
},
{
question: "How can Papermark AI help me reach more investors?",
answer:
"Papermark AI provides recommendations and analytics to help you fine-tune your pitch decks, increasing your chances of making a successful connection with potential investors.",
},
{
question: "How I as an investor can use Papermark AI?",
answer:
"VCs can utilize Papermark AI to efficiently analyze and summarize data from various pitch decks, streamlining their investment decision-making process. Search inside the pitch deck, summarise and turn it into Memo.",
},
{
question: "Can I contribute to the Papermark AI project?",
answer:
"Definitely! We welcome contributions to Papermark AI. Whether it's improving the code, adding new features, or reporting bugs, your input is highly valued.",
},
{
question: "How to summarise document with Papermark AI?",
answer:
"You can use one of the starting commands and get the summary of received docuement. If you want that documents shared with you.",
},
{
question: "How to turn Pitch Deck into Memo?",
answer:
"You can use Papermark AI and ask it to create Memo from Pitch Deck received. You can also uplaod your own Pitch deck there.",
},

// More questions...
];
Copy link
Owner

Choose a reason for hiding this comment

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

You are removing the questions. These are different from the one in the @/components/web/faq component. Ideally we make a FAQ component that takes in props with different questions and answers.

Comment on lines 23 to 39
const [menuOpen, setMenuOpen] = useState<boolean>(false);
const [isFirstClick, setIsFirstClick] = useState<boolean>(false);

<Transition
as={Fragment}
enter="transition ease-out duration-100"
enterFrom="transform opacity-0 scale-95"
enterTo="transform opacity-100 scale-100"
leave="transition ease-in duration-75"
leaveFrom="transform opacity-100 scale-100"
leaveTo="transform opacity-0 scale-95"
>
<Menu.Items
className="absolute left-0 z-10 mt-2 w-full origin-top-right rounded-md bg-white shadow-lg ring-1 ring-black ring-opacity-5 focus:outline-none"
key={option}
>
<div className="">
{options.map((optionItem) => (
<Menu.Item key={optionItem}>
{({ active }) => (
<button
onClick={() => setOption(optionItem)}
className={classNames(
active ? "bg-gray-100 text-gray-900" : "text-gray-700",
option === optionItem ? "bg-gray-200" : "",
"flex w-full items-center justify-between space-x-2 px-4 py-2 text-left text-sm",
)}
>
<span>{optionItem}</span>
{option === optionItem ? (
<CheckIcon className="text-bold h-4 w-4" />
) : null}
</button>
)}
</Menu.Item>
))}
</div>
</Menu.Items>
</Transition>
</Menu>
);
const handleMenuStateChange = (open: boolean) => {
if (isFirstClick) {
setMenuOpen(true); // Keep the dropdown open on the first click
return;
}

// If the menu is closed, reset the isFirstClick state
if (!open) {
setIsFirstClick(false);
setMenuOpen(false); // Ensure the dropdown is closed
} else {
setMenuOpen(true); // Open the dropdown
}
};
Copy link
Owner

Choose a reason for hiding this comment

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

No needed. same as above

}
};
return (
<Select open={menuOpen} onOpenChange={handleMenuStateChange}>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<Select open={menuOpen} onOpenChange={handleMenuStateChange}>
<Select>

@doppedheart doppedheart requested a review from mfts July 31, 2024 07:55
Copy link
Owner

@mfts mfts left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for your effort!

Copy link

vercel bot commented Jul 31, 2024

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

Name Status Preview Comments Updated (UTC)
papermark ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2024 10:08am

@mfts mfts merged commit 73e46d1 into mfts:main Jul 31, 2024
1 check passed
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.

Replace @headlessui/react with all shadcn/ui library components for better efficiency
2 participants