-
Notifications
You must be signed in to change notification settings - Fork 2
fix(packages): implement multiple fixes to new components #43
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
Changes from all commits
5c73635
3345e25
785114f
eef5518
591a9c6
111ced6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,9 +25,22 @@ export function FinalityProviderItem({ bsnId, bsnName, bsnLogoUrl, address, prov | |
if (!provider) return null; | ||
|
||
const renderBsnLogo = () => { | ||
if (!bsnLogoUrl) return null; | ||
if (bsnLogoUrl) { | ||
return <Avatar url={bsnLogoUrl} alt={bsnName} variant="rounded" size="tiny" className="mr-1" />; | ||
} | ||
|
||
const placeholderLetter = bsnName?.charAt(0).toUpperCase() || "?"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will address this refactor in a follow up PR |
||
|
||
return <Avatar url={bsnLogoUrl} alt={bsnName} variant="rounded" size="tiny" className="mr-1" />; | ||
return ( | ||
<Avatar variant="rounded" size="tiny" className="mr-1"> | ||
<Text | ||
as="span" | ||
className="inline-flex h-full w-full items-center justify-center bg-secondary-main text-xs text-accent-contrast" | ||
> | ||
{placeholderLetter} | ||
</Text> | ||
</Avatar> | ||
); | ||
}; | ||
|
||
const shortenAddress = (value: string): string => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method isn’t specific to this component. It should be abstracted and moved to a utility module for common use. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will address this refactor in a follow up PR |
||
|
@@ -54,12 +67,14 @@ export function FinalityProviderItem({ bsnId, bsnName, bsnLogoUrl, address, prov | |
return ( | ||
<div className="flex flex-row items-center justify-between"> | ||
<div className="flex h-10 flex-row gap-2"> | ||
<FinalityProviderLogo | ||
logoUrl={provider.logo_url} | ||
rank={provider.rank} | ||
moniker={provider.description?.moniker} | ||
size="lg" | ||
/> | ||
<div className="shrink-0"> | ||
<FinalityProviderLogo | ||
logoUrl={provider.logo_url} | ||
rank={provider.rank} | ||
moniker={provider.description?.moniker} | ||
size="lg" | ||
/> | ||
</div> | ||
<div className="flex flex-col justify-center text-accent-primary"> | ||
{renderChainOrAddress()} | ||
<Text as="div" className="text-base font-medium text-accent-primary"> | ||
|
@@ -70,7 +85,7 @@ export function FinalityProviderItem({ bsnId, bsnName, bsnLogoUrl, address, prov | |
{onRemove ? | ||
<button | ||
onClick={() => onRemove(bsnId)} | ||
className="cursor-pointer rounded bg-accent-secondary/20 px-2 py-0.5 text-xs tracking-[0.4px] text-accent-primary" | ||
className="ml-[10px] cursor-pointer rounded bg-accent-secondary/20 px-2 py-0.5 text-xs tracking-[0.4px] text-accent-primary" | ||
> | ||
Remove | ||
</button> : null} | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,2 @@ | ||||||
export const BTC_DECIMAL_PLACES = 8; | ||||||
export const BBN_DECIMAL_PLACES = 5; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is trailing whitespace at the end of the line that should be removed.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jonybur linting |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,6 @@ | ||||||
import { Text } from "../../../components/Text"; | ||||||
import { twMerge } from "tailwind-merge"; | ||||||
import { BTC_DECIMAL_PLACES } from "../../../utils/constants"; | ||||||
|
||||||
interface TotalProps { | ||||||
total: number | string; | ||||||
|
@@ -10,16 +11,18 @@ interface TotalProps { | |||||
decimals?: number; | ||||||
} | ||||||
|
||||||
export function Total({ total, coinSymbol, hint, title = "Total", className, decimals = 8 }: TotalProps) { | ||||||
const formattedTotal = | ||||||
typeof total === "number" | ||||||
? total === 0 | ||||||
? "0" | ||||||
: (() => { | ||||||
const str = total.toFixed(decimals); | ||||||
return str.replace(/0+$/, "").replace(/\.$/, ""); | ||||||
})() | ||||||
: total; | ||||||
export function Total({ total, coinSymbol, hint, title = "Total", className, decimals = BTC_DECIMAL_PLACES }: TotalProps) { | ||||||
let formattedTotal; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable formattedTotal should be declared with an explicit type annotation (string) for better type safety and code clarity.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
if (typeof total === "number") { | ||||||
if (total === 0) { | ||||||
formattedTotal = "0"; | ||||||
} else { | ||||||
const str = total.toFixed(decimals); | ||||||
formattedTotal = str.replace(/0+$/, "").replace(/\.$/, ""); | ||||||
} | ||||||
} else { | ||||||
formattedTotal = total; | ||||||
} | ||||||
|
||||||
return ( | ||||||
<div className={twMerge("flex flex-row items-start justify-between text-accent-primary", className)}> | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is no longer needed, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will address this refactor in a follow up PR