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

Fixing Duration Style for Desktop View #36

Merged
merged 2 commits into from
Feb 18, 2025
Merged

Fixing Duration Style for Desktop View #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6457a6
fix
ahmadtaimoor-deriv Feb 18, 2025
0f15d65
fix
ahmadtaimoor-deriv Feb 18, 2025
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
20 changes: 9 additions & 11 deletions src/components/Duration/DurationController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,13 @@ export const DurationController: React.FC<DurationControllerProps> = ({
<>
<div className={isDesktop ? "flex" : ""}>
{!isDesktop && <BottomSheetHeader title="Duration" />}
<div className="px-4 py-2">
<TabList
tabs={DURATION_TYPES}
selectedValue={selectedType}
onSelect={handleTypeSelect as (value: string) => void}
variant={isDesktop ? "vertical" : "chip"}
/>
</div>
<div className={`flex-1 relative ${isDesktop ? "px-4" : "px-8"}`}>
<TabList
tabs={DURATION_TYPES}
selectedValue={selectedType}
onSelect={handleTypeSelect as (value: string) => void}
variant={isDesktop ? "vertical" : "chip"}
/>
<div className={`flex-1 relative bg-white ${isDesktop ? "px-2" : "px-8"}`}>
{selectedType === "hour" ? (
<HoursDurationValue
selectedValue={selectedValue.toString()}
Expand Down Expand Up @@ -134,8 +132,8 @@ export const DurationController: React.FC<DurationControllerProps> = ({

if (isDesktop) {
return (
<DesktopTradeFieldCard>
<div className="w-[480px]">{content}</div>
<DesktopTradeFieldCard className="p-0">
<div className="w-[368px]">{content}</div>
</DesktopTradeFieldCard>
);
}
Expand Down
79 changes: 45 additions & 34 deletions src/components/Duration/DurationField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,64 +1,75 @@
import React, { useState, useRef } from "react";
import { useTradeStore } from "@/stores/tradeStore";
import { useBottomSheetStore } from "@/stores/bottomSheetStore";
import { useDeviceDetection } from "@/hooks/useDeviceDetection";
import TradeParam from "@/components/TradeFields/TradeParam";
import { DurationController } from "./DurationController";
import { Popover } from "@/components/ui/popover";
import React, { useState, useRef } from "react"
import { useTradeStore } from "@/stores/tradeStore"
import { useBottomSheetStore } from "@/stores/bottomSheetStore"
import { useDeviceDetection } from "@/hooks/useDeviceDetection"
import TradeParam from "@/components/TradeFields/TradeParam"
import { DurationController } from "./DurationController"
import { Popover } from "@/components/ui/popover"
import { DesktopTradeFieldCard } from "@/components/ui/desktop-trade-field-card"

interface DurationFieldProps {
className?: string;
className?: string
}

export const DurationField: React.FC<DurationFieldProps> = ({ className }) => {
const { duration } = useTradeStore();
const { setBottomSheet } = useBottomSheetStore();
const { isDesktop } = useDeviceDetection();
const [isOpen, setIsOpen] = useState(false);
const popoverRef = useRef<{ isClosing: boolean }>({ isClosing: false });
const { duration } = useTradeStore()
const { setBottomSheet } = useBottomSheetStore()
const { isDesktop } = useDeviceDetection()
const [isOpen, setIsOpen] = useState(false)
const popoverRef = useRef<{ isClosing: boolean }>({ isClosing: false })

const handleClick = () => {
if (isDesktop) {
if (!popoverRef.current.isClosing) {
setIsOpen(!isOpen);
setIsOpen(!isOpen)
}
} else {
setBottomSheet(true, "duration", "470px");
setBottomSheet(true, "duration", "470px")
}
};
}

const handleClose = () => {
popoverRef.current.isClosing = true;
setIsOpen(false);
popoverRef.current.isClosing = true
setIsOpen(false)
// Reset after a longer delay
setTimeout(() => {
popoverRef.current.isClosing = false;
}, 300); // 300ms should be enough for the animation to complete
};
popoverRef.current.isClosing = false
}, 300) // 300ms should be enough for the animation to complete
}

return (
<div className="relative">
<TradeParam
label="Duration"
value={duration}
onClick={handleClick}
className={className}
/>

{isDesktop ? (
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider extracting the conditional wrapper of the "TradeParam" component to reduce JSX duplication.

Consider extracting the conditional wrapper into its own component or helper function so that you only render the <TradeParam> once. This reduces the duplicated props and branching in your JSX.

For example, you can create a simple wrapper component:

const ConditionalWrapper: React.FC<{ children: React.ReactNode }> = ({ children }) =>
  isDesktop ? <DesktopTradeFieldCard isSelected={isOpen}>{children}</DesktopTradeFieldCard> : <>{children}</>;

return (
  <div className="relative">
    <ConditionalWrapper>
      <TradeParam
        label="Duration"
        value={duration}
        onClick={handleClick}
        className={className}
      />
    </ConditionalWrapper>
    {isDesktop && isOpen && (
      <Popover
        isOpen={isOpen}
        onClose={handleClose}
        style={{
          position: "absolute",
          right: "100%",
          top: "-8px",
          marginRight: "16px",
        }}
      >
        <DurationController onClose={handleClose} />
      </Popover>
    )}
  </div>
);

This approach retains all functionality while reducing duplication and simplifying the rendering logic.

<DesktopTradeFieldCard isSelected={isOpen}>
<TradeParam
label="Duration"
value={duration}
onClick={handleClick}
className={className}
/>
</DesktopTradeFieldCard>
) : (
<TradeParam
label="Duration"
value={duration}
onClick={handleClick}
className={className}
/>
)}
{isDesktop && isOpen && (
<Popover
isOpen={isOpen}
onClose={handleClose}
style={{
position: 'absolute',
right: '100%',
top: '-8px',
marginRight: '16px'
position: "absolute",
right: "100%",
top: "-8px",
marginRight: "16px",
}}
>
<DurationController onClose={handleClose} />
</Popover>
)}
</div>
);
};
)
}
2 changes: 1 addition & 1 deletion src/components/MarketInfo/MarketInfo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const MarketInfo: React.FC<MarketInfoProps> = ({
data-id="market-info"
onClick={onClick}
>
<div className="flex items-center gap-4 px-4 py-3">
<div className="flex items-center gap-4 px-4 py-3 bg-black/[0.04]">
{selectedMarket && (
<div className="w-8 h-8 flex items-center justify-center">
<MarketIcon
Expand Down
4 changes: 2 additions & 2 deletions src/components/ui/__tests__/desktop-trade-field-card.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('DesktopTradeFieldCard', () => {
);

const card = container.firstChild as HTMLElement;
expect(card).toHaveClass('bg-white', 'rounded-lg', 'p-2');
expect(card).toHaveClass('rounded-lg', 'p-2','bg-[rgba(246,247,248,1)]','border','border-transparent');
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Test the isSelected style

Add a test case to verify the styles when the isSelected prop is true. This ensures the visual feedback for the selected state is correctly applied.

Suggested implementation:

  it('merges custom className with default styles', () => {
    // existing test code...
  });

  // New test to verify styles when isSelected is true
  it('applies isSelected style when isSelected prop is true', () => {
    // Render the component with the isSelected prop set to true.
    const { container } = render(<DesktopTradeFieldCard isSelected={true} />);
    const card = container.firstChild as HTMLElement;
    // Expect the card to have default styles plus the selected border style.
    expect(card).toHaveClass('rounded-lg', 'p-2', 'bg-[rgba(246,247,248,1)]', 'border', 'border-blue-500');
  });

Make sure that the component DesktopTradeFieldCard is imported in this test file if it isn't already. For example:

<<<<<<< SEARCH
// (No import of DesktopTradeFieldCard)

import DesktopTradeFieldCard from '../DesktopTradeFieldCard';

REPLACE

Also ensure that test utilities like render are imported from the React Testing Library.

});

it('merges custom className with default styles', () => {
Expand All @@ -31,6 +31,6 @@ describe('DesktopTradeFieldCard', () => {
);

const card = container.firstChild as HTMLElement;
expect(card).toHaveClass('bg-white p-2 rounded-lg');
expect(card).toHaveClass('bg-[rgba(246,247,248,1)] rounded-lg p-2 border border-transparent custom-class');
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Test the error style

It's important to also test the error styles. Add a test case where the error prop is true to ensure the visual feedback for errors is correctly displayed.

Suggested implementation:

  it('applies error styles when error prop is true', () => {
    const { container } = render(<DesktopTradeFieldCard error />);
    const card = container.firstChild as HTMLElement;
    expect(card).toHaveClass('bg-[rgba(246,247,248,1)]', 'rounded-lg', 'p-2', 'border', 'border-red-500');
  });
});

Make sure that the DesktopTradeFieldCard component applies "border-red-500" (or the correct error class for your design) when the error prop is true. Adjust expected class names in the test if your design uses different classes.

});
});
4 changes: 1 addition & 3 deletions src/components/ui/desktop-trade-field-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,15 @@ interface DesktopTradeFieldCardProps {

export const DesktopTradeFieldCard = ({ children, className, isSelected, error }: DesktopTradeFieldCardProps) => {
return (
<div className="bg-white p-2 rounded-lg">
<div
className={cn(
"bg-[rgba(246,247,248,1)] rounded-lg p-2 border border-transparent",
isSelected && "border-black",
isSelected && "border-primary",
error && "border-red-500 bg-[rgba(230,25,14,0.08)]",
className
)}
>
{children}
</div>
</div>
);
};
13 changes: 7 additions & 6 deletions src/components/ui/tab-list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,18 @@ const VerticalTabList: React.FC<BaseTabListProps> = ({
onSelect,
}) => {
return (
<div className="w-48 border-r border-gray-200">
<div className="w-28 bg-[#F6F7F8]">
{tabs.map(({ label, value }) => (
<button
key={value}
onClick={() => onSelect(value)}
className={`
w-full text-left py-2 px-4 transition-colors
${
selectedValue === value
? "bg-gray-100 font-bold"
: "hover:bg-gray-50"
relative w-full text-left py-3 px-6 transition-colors font-ibm-plex text-base leading-6 font-normal
text-primary cursor-pointer
before:absolute before:left-0 before:top-0 before:bottom-0 before:w-[4px]
${selectedValue === value
? "bg-white before:bg-black"
: "hover:bg-gray-50 before:bg-transparent"
}
`}
>
Expand Down
Loading