Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(app): add modal to add question #390

Merged
merged 19 commits into from
Dec 8, 2022
Merged

Conversation

AdiPol1359
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Dec 7, 2022

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

Name Status Preview Updated
devfaq ✅ Ready (Inspect) Visit Preview Dec 8, 2022 at 3:36PM (UTC)

@AdiPol1359 AdiPol1359 marked this pull request as ready for review December 8, 2022 12:21

export const AddQuestionModal = (props: ComponentProps<typeof BaseModal>) => (
<BaseModal {...props}>
<h3 className="text-center text-xl font-bold uppercase text-primary">Nowe pytanie</h3>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<h3 className="text-center text-xl font-bold uppercase text-primary">Nowe pytanie</h3>
<h2 className="text-center text-xl font-bold uppercase text-primary">Nowe pytanie</h2>


export const AddQuestionModal = (props: ComponentProps<typeof BaseModal>) => (
<BaseModal {...props}>
<h3 className="text-center text-xl font-bold uppercase text-primary">Nowe pytanie</h3>
Copy link
Member

Choose a reason for hiding this comment

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

Brakuje jakiegoś elementu form nad tym

<option>HTML5</option>
<option>JavaScript</option>
</Select>
<Select className="w-full">
Copy link
Member

Choose a reason for hiding this comment

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

Selekty nie mają labeli

<option>Wybierz Poziom</option>
<option>junior</option>
<option>Mid</option>
<option>Senior</option>
Copy link
Member

Choose a reason for hiding this comment

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

option nie mają value


export const BaseModal = ({ isOpen, onClose, children }: BaseModalProps) => {
useEffect(() => {
isOpen && lockScroll();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
isOpen && lockScroll();
if (isOpen) {
lockScroll();
}

Copy link
Member

Choose a reason for hiding this comment

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

Nie brakuje tu else z unlockScroll ?

>
<div
className="relative h-full w-full max-w-3xl animate-show rounded-sm bg-white px-3.5 py-9 sm:h-fit sm:px-11 sm:py-20"
onClick={(event) => event.stopPropagation()}
Copy link
Member

Choose a reason for hiding this comment

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

Div z onClick? Dlaczego?


export const Select = ({ className, ...props }: SelectHTMLAttributes<HTMLSelectElement>) => (
<select
className={twMerge(
Copy link
Member

Choose a reason for hiding this comment

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

Te zaokrąglone brzegi wyglądają średnio:
Screenshot 2022-12-08 at 14 42 52

}, []);

return (
<ModalContextProvider value={{ openedModal, openModal, closeModal }}>
Copy link
Member

Choose a reason for hiding this comment

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

Tutaj tworzy się nowy obiekt przy każdym renderze i powoduje to, że cały kontekst się przerenderowuje. Brakuje useMemo

Comment on lines 42 to 49
backgroundImage: {
"select-purple": 'url("/select-purple.svg")',
},
backgroundSize: {
"select-purple": "25px",
},
backgroundPosition: {
"select-purple": "100% 50%",
Copy link
Member

Choose a reason for hiding this comment

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

Można po prostu dopisać klasę do pliku tailwind.css :)

Comment on lines 30 to 32
afterLeave={() => {
unlockScroll();
}}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
afterLeave={() => {
unlockScroll();
}}
afterLeave={unlockScroll}

>
<div
className="relative h-full w-full max-w-3xl animate-show rounded-sm bg-white px-3.5 py-9 sm:h-fit sm:px-11 sm:py-20"
onClick={(event) => event.stopPropagation()}
onClick={(event) => {
// stop triggering click event to higher component
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// stop triggering click event to higher component
// stop propagation to avoid triggering `onClick` on the backdrop behind the modal

@@ -4,7 +4,7 @@ import type { SelectHTMLAttributes } from "react";
export const Select = ({ className, ...props }: SelectHTMLAttributes<HTMLSelectElement>) => (
<select
className={twMerge(
"cursor-pointer appearance-none border-b border-primary bg-select-purple bg-no-repeat py-2 pr-6 pl-1 text-sm capitalize text-primary transition-shadow duration-100 focus:shadow-[0_0_10px] focus:shadow-primary focus:outline-0",
"select-purple cursor-pointer appearance-none rounded-none border-b border-primary py-2 pr-6 pl-1 text-base capitalize text-primary transition-shadow duration-100 focus:shadow-[0_0_10px] focus:shadow-primary focus:outline-0",
Copy link
Member

Choose a reason for hiding this comment

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

Brakuje chyba appearance-none

@typeofweb typeofweb merged commit 8adc36b into develop Dec 8, 2022
@typeofweb typeofweb deleted the 389-add-question-modal branch December 8, 2022 15:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants