Skip to content
This repository has been archived by the owner on Aug 9, 2023. It is now read-only.

refactor: button component #66

Merged
merged 6 commits into from
Jul 29, 2023
Merged

refactor: button component #66

merged 6 commits into from
Jul 29, 2023

Conversation

PwFaze
Copy link
Contributor

@PwFaze PwFaze commented Jul 22, 2023

Change made

  • Bug fixes
  • New features
  • Breaking changes

Describe what you have done

New Features

Fix

Others

@vercel
Copy link

vercel bot commented Jul 22, 2023

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

Name Status Preview Comments Updated (UTC)
rpkm66-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 29, 2023 2:10pm

Copy link
Member

@shalluv shalluv left a comment

Choose a reason for hiding this comment

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

Please take a look at clsx. I think it will make it easier to construct className.

.env.example Outdated Show resolved Hide resolved

const DefaultButton = ({ content, additionalStyle, onClick, disabled }) => {
const buttonClassName =
`max-w-full w-40 mx-auto items-center py-2 text-white transition-all duration-500 hover:ring-8 disabled:cursor-not-allowed disabled:ring-0 ` +
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to explicitly declare a variable, just write to the className attribute of the button.

Copy link
Member

Choose a reason for hiding this comment

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

You can also use import clsx from 'clsx' to concatenate class.

src/components/DefaultButton.tsx Outdated Show resolved Hide resolved

const DefaultButton = ({ content, additionalStyle, onClick, disabled }) => {
const buttonClassName =
`max-w-full w-40 mx-auto items-center py-2 text-white transition-all duration-500 hover:ring-8 disabled:cursor-not-allowed disabled:ring-0 ` +
Copy link
Member

Choose a reason for hiding this comment

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

You can also use import clsx from 'clsx' to concatenate class.

Comment on lines 151 to 158
{/* <button
className="mx-auto rounded-lg bg-pink-400 px-3 py-2 text-xl text-white ring-4 ring-pink-400/30 transition-all duration-500 enabled:hover:ring-8 disabled:cursor-not-allowed disabled:bg-pink-300"
onClick={() => openModal('modal-leave-group')}
disabled={!isAuthenticated || group?.leaderID === user?.id}
>
ออกจากกลุ่ม
</button>
</button> */}
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Uncomment and delete these after finished

Copy link
Member

@boomchanotai boomchanotai left a comment

Choose a reason for hiding this comment

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

Cool! Please add typescript for button for Button Component

@@ -0,0 +1,15 @@
import React from 'react';

const Button = ({ content, additionalStyle, onClick, disabled }) => {
Copy link
Member

Choose a reason for hiding this comment

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

Please add typescript for this component's props

@@ -46,6 +46,7 @@ const AuthProvider = ({ children }: { children: ReactNode }) => {
const token = localStorage.getItem('token');
if (token) {
const userProfile = await getUserProfile();
console.log(userProfile);
Copy link
Member

Choose a reason for hiding this comment

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

Please delete console.log if it not necessary

Copy link
Member

@boomchanotai boomchanotai left a comment

Choose a reason for hiding this comment

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

Great Job!

@boomchanotai boomchanotai merged commit 868944e into dev Jul 29, 2023
2 checks passed
@leomotors leomotors deleted the ButtonComponent branch August 3, 2023 09:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants