-
Notifications
You must be signed in to change notification settings - Fork 14
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
Farabi/fix contract details page desktop #52
Farabi/fix contract details page desktop #52
Conversation
Reviewer's Guide by SourceryThis pull request refactors the device detection logic to use orientation detection instead of device type detection. It replaces Sequence diagram for rendering ContractDetailsPagesequenceDiagram
participant CDP as ContractDetailsPage
participant OS as useOrientationStore
participant DCDP as DesktopContractDetailsPage
participant MC as MobileContractDetailsPage
CDP->OS: isLandscape
alt isLandscape is false
CDP->MC: Render MobileContractDetailsPage
else isLandscape is true
CDP->DCDP: Render DesktopContractDetailsPage
end
Updated class diagram for device detection logicclassDiagram
class useDeviceDetection {
- isMobile: boolean
- isDesktop: boolean
}
note for useDeviceDetection "Deprecated"
class useOrientationStore {
+ isLandscape: boolean
+ setIsLandscape(isLandscape: boolean): void
}
useOrientationStore --|> orientationChangeEvent : listens to
Updated class diagram for DesktopContractDetailsPageclassDiagram
class DesktopContractDetailsPage {
-className: string
}
note for DesktopContractDetailsPage "Adjusted styles and layout"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
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.
Hey @farabi-deriv - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider renaming
useOrientationStore
touseOrientation
for brevity and clarity. - Ensure that the
useOrientationStore
provides a default value or handles initial undefined states to prevent unexpected behavior.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Deploying champion-trader with
|
Latest commit: |
918677a
|
Status: | ✅ Deploy successful! |
Preview URL: | https://2394cfa6.champion-trader.pages.dev |
Branch Preview URL: | https://farabi-fix-contract-details.champion-trader.pages.dev |
This pull request focuses on updating the device detection logic to use orientation detection instead. The most important changes include replacing
useDeviceDetection
withuseOrientationStore
across various components and adjusting the layout and styles accordingly.Changes to device detection logic:
src/components/ContractDetailsChart/ContractDetailsChart.tsx
: ReplaceduseDeviceDetection
withuseOrientationStore
and updated the layout to useisLandscape
instead ofisDesktop
. [1] [2]src/layouts/MainLayout/MainLayout.tsx
: RemovedisDesktop
fromuseDeviceDetection
and replaced it withisLandscape
fromuseOrientationStore
for layout adjustments. [1] [2]src/screens/ContractDetailsPage/ContractDetailsPage.tsx
: ReplaceduseDeviceDetection
withuseOrientationStore
and updated the conditional rendering logic to useisLandscape
instead ofisMobile
. [1] [2]Changes to layout and styles:
src/screens/ContractDetailsPage/DesktopContractDetailsPage.tsx
: Adjusted styles and layout for the desktop contract details page, including removing the full height class and modifying padding and margins. [1] [2]src/screens/ContractDetailsPage/__tests__/DesktopContractDetailsPage.test.tsx
: Updated tests to reflect the new layout and style changes for the desktop contract details page.Summary by Sourcery
Enhancements: