fix(packages): implement multiple fixes to new components #43
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,10 +65,3 @@ export const SingleMax: Story = { | |
| max: 1, | ||
| }, | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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. There is trailing whitespace at the end of the line that should be removed.
Suggested change
Copilot uses AI. Check for mistakes. |
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 = BTC_DECIMAL_PLACES }: TotalProps) { | ||||||
| let formattedTotal; | ||||||
|
There was a problem hiding this comment. 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. |
||||||
| 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)}> | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,30 +3,27 @@ | |
| import { Dialog, MobileDialog, DialogBody, DialogFooter, DialogHeader } from "@/components/Dialog"; | ||
| import { Heading } from "@/components/Heading"; | ||
| import { Text } from "@/components/Text"; | ||
| import { useIsMobile } from "@/hooks/useIsMobile"; | ||
| import { PropsWithChildren, ReactNode } from "react"; | ||
| import { twMerge } from "tailwind-merge"; | ||
|
|
||
| const toKebabCase = (str: string): string => { | ||
|
There was a problem hiding this comment. The toKebabCase function should be moved to a utilities file rather than being defined inline in this component, as it could be reused elsewhere in the codebase. Copilot uses AI. Check for mistakes. There was a problem hiding this comment. @jonybur this comment make sense to me |
||
| return str | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9]+/g, '-') | ||
| .replace(/^-+|-+$/g, ''); | ||
|
||
| }; | ||
|
|
||
| type DialogComponentProps = Parameters<typeof Dialog>[0]; | ||
|
|
||
| interface ResponsiveDialogProps extends DialogComponentProps { | ||
| children?: ReactNode; | ||
| } | ||
|
|
||
| const WINDOW_BREAKPOINT = 640; | ||
|
There was a problem hiding this comment. this should be moved to a centralised place too. sounds like a constant value which you can use across the codebase There was a problem hiding this comment. Will address this refactor in a follow up PR |
||
|
|
||
| function ResponsiveDialog({ className, ...restProps }: ResponsiveDialogProps) { | ||
| const isMobileView = useIsMobile(WINDOW_BREAKPOINT); | ||
| const DialogComponent = isMobileView ? MobileDialog : Dialog; | ||
|
|
||
| return <DialogComponent {...restProps} className={twMerge("w-[41.25rem] max-w-full", className)} />; | ||
|
|
@@ -107,7 +104,7 @@ | |
| <div className="grid grid-cols-2 items-center gap-4 rounded bg-primary-contrast p-4"> | ||
| <div className="flex flex-col gap-3"> | ||
| {bsns.map((bsnItem, index) => ( | ||
| <div key={`bsn-${toKebabCase(bsnItem.name)}-${index}`} className="flex w-full items-center justify-center gap-2 py-1"> | ||
|
There was a problem hiding this comment. why not use the bsn id? There was a problem hiding this comment. Will address this refactor in a follow up PR |
||
| {bsnItem.icon} | ||
| <Text variant="body2" className="font-medium"> | ||
| {bsnItem.name} | ||
|
|
@@ -117,7 +114,7 @@ | |
| </div> | ||
| <div className="flex flex-col gap-3"> | ||
| {finalityProviders.map((fpItem, index) => ( | ||
| <div key={`fp-${toKebabCase(fpItem.name)}-${index}`} className="flex w-full items-center justify-center gap-2 py-1"> | ||
| {fpItem.icon} | ||
| <Text variant="body2" className="font-medium"> | ||
| {fpItem.name} | ||
|
|
@@ -150,23 +147,21 @@ | |
| </Heading> | ||
| <Text variant="body2" className="text-secondary"> | ||
| 1. No third party possesses your staked BTC. You are the only one who can unbond and withdraw your stake. | ||
|
There was a problem hiding this comment. [nitpick] The numbered text content should be extracted to constants or props to improve maintainability and potential internationalization support. Copilot uses AI. Check for mistakes. |
||
| </Text> | ||
| <Text variant="body2" className="text-secondary"> | ||
| 2. Your stake will first be sent to Babylon Genesis for verification (~20 seconds), then you will be | ||
| prompted to submit it to the Bitcoin ledger. It will be marked as 'Pending' until it receives 10 | ||
| Bitcoin confirmations. | ||
| </Text> | ||
| </div> | ||
| </DialogBody> | ||
| <DialogFooter className="flex flex-col gap-4 pb-8 pt-0 sm:flex-row"> | ||
| <Button variant="contained" color="primary" onClick={onProceed} className="w-full sm:flex-1 sm:order-2" disabled={processing}> | ||
| {processing ? "Processing..." : "Proceed to Signing"} | ||
| </Button> | ||
| <Button variant="outlined" color="primary" onClick={onClose} className="w-full sm:flex-1 sm:order-1"> | ||
| Cancel | ||
| </Button> | ||
| </DialogFooter> | ||
| </ResponsiveDialog> | ||
| ); | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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