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.

Sign up for GitHub

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

ButtonLink: extended component as Link #184

Merged
merged 6 commits into from
Jan 26, 2024
Merged

ButtonLink: extended component as Link #184

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
38740e8
ButtonLink: extended component as Link
ioay Jan 18, 2024
007ec0f
Merge branch 'main' into extend-buttonlink-component
ioay Jan 23, 2024
27ff4bc
ButtonLink styles update
ioay Jan 23, 2024
03a71fe
Adden theme for Link
ioay Jan 25, 2024
3b07f39
Merge branch 'main' into extend-buttonlink-component
ioay Jan 26, 2024
a163c9d
Removed textDecoration prop from Button theme
ioay Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions dapp/src/components/shared/ButtonLink/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React from "react"
import { Button, ButtonProps, Icon } from "@chakra-ui/react"
import { Button, ButtonProps, Icon, Link, LinkProps } from "@chakra-ui/react"
import { ArrowUpRight } from "#/static/icons"

type ButtonLinkProps = ButtonProps & {
icon?: typeof Icon
iconColor?: string
}
type ButtonLinkProps = ButtonProps &
LinkProps & {
icon?: typeof Icon
iconColor?: string
}

export default function ButtonLink({
children,
Expand All @@ -16,6 +17,7 @@ export default function ButtonLink({
}: ButtonLinkProps) {
return (
<Button
as={Link}
variant={variant}
justifyContent="flex-start"
borderRadius="md"
Expand Down
9 changes: 9 additions & 0 deletions dapp/src/theme/Button.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ export const buttonTheme: ComponentSingleStyleConfig = {
}
return defaultStyles
},
ghost: {
_hover: {
bg: "transparent",
},
_active: {
bg: "transparent",
},
},
Comment on lines +58 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need it also? I don't see designs on Figma for a ghost button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, look at https://www.figma.com/file/OUdSfHsmgV8EJpWxAzXjl5/Design-Tasks?node-id=4151%3A22276&mode=dev,

Screenshot 2024-01-23 at 17 24 46

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I see it. But there is no such button in the styleguide. That's why I wonder if we need it. The task refers to the extension of the component ButtonLink as Link (extension of functionality). Therefore, it seems to me that at this PR we do not need to add it if it is not confirmed by Sorin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are not many things that are stylized, but as we agreed - we style it based on the project. Let's leave it as is, if the design changes - we will change the style. I have the impression that we are discussing here matters that are of no great importance.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you understood it that way, I'm sorry. My question was just out of curiosity because I could not find such a variant in styleguide. I wasn't sure if the topic hadn't already been discussed with Sorin before and I missed something.

Yes, of course, we can update this later, it's not a problem. I just noticed that if it's not required in the task at all, we don't need to add it now and deal with it later when needed.

// FIXME: It should be removed and replaced by solid/outline variants
card: ({ colorScheme }: StyleFunctionProps) => {
const defaultStyles = {
Expand Down Expand Up @@ -85,6 +93,7 @@ export const buttonTheme: ComponentSingleStyleConfig = {
_hover: {
borderColor: "white",
bg: "opacity.white.6",
textDecoration: "underline",
kkosiorowska marked this conversation as resolved.
Show resolved Hide resolved
},
_disabled: {
color: "grey.200",
Expand Down
11 changes: 11 additions & 0 deletions dapp/src/theme/Link.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { defineStyle, defineStyleConfig } from "@chakra-ui/react"

const baseStyle = defineStyle({
_hover: {
textDecoration: "none",
},
})

export const linkTheme = defineStyleConfig({
baseStyle,
})
2 changes: 2 additions & 0 deletions dapp/src/theme/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { formErrorTheme } from "./FormError"
import { tabsTheme } from "./Tabs"
import { spinnerTheme } from "./Spinner"
import { tableTheme } from "./Table"
import { linkTheme } from "./Link"

const defaultTheme = {
colors,
Expand Down Expand Up @@ -47,6 +48,7 @@ const defaultTheme = {
FormError: formErrorTheme,
Heading: headingTheme,
Input: inputTheme,
Link: linkTheme,
Modal: modalTheme,
Sidebar: sidebarTheme,
Spinner: spinnerTheme,
Expand Down